feat!: introduce epochs (#1691)

* docs: modify epochs ADR to capture latest design (#1668) * modified ADR to capture the epoch design * feat!: introduce epochs (#1660) * cleanup ./changelog entries * rebase * fix!: Validation of SlashAcks fails due to marshaling to Bech32 (backport #1570) (#1577) fix!: Validation of SlashAcks fails due to marshaling to Bech32 (#1570) * add different Bech32Prefix for consumer and provider * separate app encoding and params * remove ConsumerValPubKey from ValidatorConfig * update addresses in tests * make SlashAcks consistent across chains * add comments for clarity * Regenerate traces * Fix argument order * set bech32prefix for provider to cosmos * add changelog entries * add consumer-double-downtime e2e test * update nightly-e2e workflow * fix typo * add consumer-double-downtime to testConfigs * remove changes on provider * skip invalid SlashAcks * seal the config * clear the outstanding downtime flag for new vals * add info on upgrading to v4.0.0 * fix upgrade handler * fix changeover e2e test * Update tests/e2e/config.go Co-authored-by: Philip Offtermatt <57488781+p-offtermatt@users.noreply.github.com> * Update tests/e2e/config.go Co-authored-by: Philip Offtermatt <57488781+p-offtermatt@users.noreply.github.com> * add AccountPrefix to ChainConfig * fix docstrings * update AccountAddressPrefix in app.go * fix consumer-misb e2e test --------- Co-authored-by: Philip Offtermatt <p.offtermatt@gmail.com> Co-authored-by: Simon Noetzlin <simon.ntz@gmail.com> Co-authored-by: Philip Offtermatt <57488781+p-offtermatt@users.noreply.github.com> (cherry picked from commit 8604692) Co-authored-by: Marius Poke <marius.poke@posteo.de> * docs: update changelog for v4.0.0 (#1578) update changelog * docs: prepare for v4.0.0 (#1581) * unclog build * update release notes * update release date * added proto declaration * temp commit * temp commit * more changes * first commit * add param and fix tests * reduce epoch size for e2e * clean up * mbt fix * fix diff bug * cleaning up * cleaning up * cleaning up * cleaning up * cleaning up * cleaning up * added more tests * more fixes * nit fixes * cleaning up * increase downtime by one block * fix logs * took into account Marius' comments * tiny fixes * Update x/ccv/provider/keeper/params.go Co-authored-by: Simon Noetzlin <simon.ntz@gmail.com> * use Bech32 addresses as keys for maps * refactor nextBlocks(epoch) to nextEpoch * fixed comment * Remove new block creation during consumer chain setup * Revert "Remove new block creation during consumer chain setup" This reverts commit 85a52b7. * added simple param test * added upper bound and addressed a comment * Add another edge case for diffing * used smarted solution (based on Philip's comment) for diffing validators * refactor!: remove key-assignment replacements (#1672) * initial commit * removed KeyAssignmentReplacementsKey * refactor: simplify key-assignment logic (#1684) * fixed typo: depreciated to deprecated --------- Co-authored-by: Marius Poke <marius.poke@posteo.de> * add the epoch param in the docs --------- Co-authored-by: mpoke <marius.poke@posteo.de> Co-authored-by: mergify[bot] <37929162+mergify[bot]@users.noreply.github.com> Co-authored-by: Simon Noetzlin <simon.ntz@gmail.com> Co-authored-by: Philip Offtermatt <p.offtermatt@gmail.com> * test: Add epochs to MBT (#1676) * cleanup ./changelog entries * rebase * fix!: Validation of SlashAcks fails due to marshaling to Bech32 (backport #1570) (#1577) fix!: Validation of SlashAcks fails due to marshaling to Bech32 (#1570) * add different Bech32Prefix for consumer and provider * separate app encoding and params * remove ConsumerValPubKey from ValidatorConfig * update addresses in tests * make SlashAcks consistent across chains * add comments for clarity * Regenerate traces * Fix argument order * set bech32prefix for provider to cosmos * add changelog entries * add consumer-double-downtime e2e test * update nightly-e2e workflow * fix typo * add consumer-double-downtime to testConfigs * remove changes on provider * skip invalid SlashAcks * seal the config * clear the outstanding downtime flag for new vals * add info on upgrading to v4.0.0 * fix upgrade handler * fix changeover e2e test * Update tests/e2e/config.go Co-authored-by: Philip Offtermatt <57488781+p-offtermatt@users.noreply.github.com> * Update tests/e2e/config.go Co-authored-by: Philip Offtermatt <57488781+p-offtermatt@users.noreply.github.com> * add AccountPrefix to ChainConfig * fix docstrings * update AccountAddressPrefix in app.go * fix consumer-misb e2e test --------- Co-authored-by: Philip Offtermatt <p.offtermatt@gmail.com> Co-authored-by: Simon Noetzlin <simon.ntz@gmail.com> Co-authored-by: Philip Offtermatt <57488781+p-offtermatt@users.noreply.github.com> (cherry picked from commit 8604692) Co-authored-by: Marius Poke <marius.poke@posteo.de> * docs: update changelog for v4.0.0 (#1578) update changelog * docs: prepare for v4.0.0 (#1581) * unclog build * update release notes * update release date * added proto declaration * temp commit * temp commit * more changes * first commit * add param and fix tests * reduce epoch size for e2e * clean up * mbt fix * fix diff bug * cleaning up * cleaning up * cleaning up * cleaning up * cleaning up * cleaning up * added more tests * more fixes * nit fixes * cleaning up * increase downtime by one block * fix logs * took into account Marius' comments * tiny fixes * Update x/ccv/provider/keeper/params.go Co-authored-by: Simon Noetzlin <simon.ntz@gmail.com> * use Bech32 addresses as keys for maps * refactor nextBlocks(epoch) to nextEpoch * Start adding epochs * Adjust tests for epochs * Use invariant script instead of handwriting Makefile * Fix key assignment valset invariant * Add better run_invariants script * Start adding epochs from trace into driver * Remove new block creation during consumer chain setup * Adjust model for epochs * Take into account comments * Revert changes to actions.go * Revert changes to x/ * Remove unused listMul * Advance time by epochLength instead of 1 second * Indent condition and clarify EndProviderEpoch --------- Co-authored-by: mpoke <marius.poke@posteo.de> Co-authored-by: insumity <karolos@informal.systems> Co-authored-by: mergify[bot] <37929162+mergify[bot]@users.noreply.github.com> Co-authored-by: Simon Noetzlin <simon.ntz@gmail.com> * added changelogs * rebase and fix compatibility test * Update docs/docs/adrs/adr-014-epochs.md Co-authored-by: Philip Offtermatt <57488781+p-offtermatt@users.noreply.github.com> * Update docs/docs/adrs/adr-014-epochs.md Co-authored-by: Philip Offtermatt <57488781+p-offtermatt@users.noreply.github.com> * nit change in test * removed blocks per epoch upper limit --------- Co-authored-by: mpoke <marius.poke@posteo.de> Co-authored-by: mergify[bot] <37929162+mergify[bot]@users.noreply.github.com> Co-authored-by: Simon Noetzlin <simon.ntz@gmail.com> Co-authored-by: Philip Offtermatt <p.offtermatt@gmail.com> Co-authored-by: Philip Offtermatt <57488781+p-offtermatt@users.noreply.github.com>
cosmos · Mar 12, 2024 · 653139a · 653139a
1 parent a9afa66
commit 653139a
Show file tree

Hide file tree

Showing 46 changed files with 2,420 additions and 674 deletions.
diff --git a/.changelog/unreleased/features/provider/1516-introduce-epochs.md b/.changelog/unreleased/features/provider/1516-introduce-epochs.md
@@ -0,0 +1,3 @@
+- Introduce epochs (i.e., send a VSCPacket every X blocks instead of in every
+  block) so that we reduce the cost of relaying IBC packets needed for ICS.
+  ([\#1516](https://github.com/cosmos/interchain-security/pull/1516))
diff --git a/.changelog/unreleased/state-breaking/provider/1516-introduce-epochs.md b/.changelog/unreleased/state-breaking/provider/1516-introduce-epochs.md
@@ -0,0 +1,3 @@
+- Introduce epochs (i.e., send a VSCPacket every X blocks instead of in every
+  block) so that we reduce the cost of relaying IBC packets needed for ICS.
+  ([\#1516](https://github.com/cosmos/interchain-security/pull/1516))
diff --git a/Makefile b/Makefile
@@ -132,6 +132,8 @@ verify-models:
 	quint test tests/mbt/model/ccv_test.qnt;\
 	quint test tests/mbt/model/ccv_model.qnt;\
 	quint run --invariant "all{ValidatorUpdatesArePropagatedInv,ValidatorSetHasExistedInv,SameVscPacketsInv,MatureOnTimeInv,EventuallyMatureOnProviderInv}" tests/mbt/model/ccv_model.qnt --max-steps 200 --max-samples 200
+	cd tests/mbt/model;\
+	../run_invariants.sh
 
 
 

diff --git a/docs/docs/adrs/adr-001-key-assignment.md b/docs/docs/adrs/adr-001-key-assignment.md
@@ -7,6 +7,7 @@ title: Key Assignment
 
 ## Changelog
 * 2022-12-01: Initial Draft
+* 2024-03-01: Updated to take into account they key-assigment-replacement deprecation.
 
 ## Status
 
@@ -30,10 +31,6 @@ ConsumerValidatorsBytePrefix | len(chainID) | chainID | providerConsAddress -> c
 ```golang
 ValidatorsByConsumerAddrBytePrefix | len(chainID) | chainID | consumerConsAddress -> providerConsAddress
 ```
-- `KeyAssignmentReplacements` - Stores the key assignments that need to be replaced in the current block. Needed to apply the key assignments received in a block to the validator updates sent to the consumer chains.
-```golang
-KeyAssignmentReplacementsBytePrefix | len(chainID) | chainID | providerConsAddress -> abci.ValidatorUpdate{PubKey: oldConsumerKey, Power: currentPower},
-```
 - `ConsumerAddrsToPrune` - Stores the mapping from VSC ids to consumer validators addresses. Needed for pruning `ValidatorByConsumerAddr`. 
 ```golang
 ConsumerAddrsToPruneBytePrefix | len(chainID) | chainID | vscID -> []consumerConsAddresses
@@ -67,20 +64,6 @@ if _, consumerRegistered := GetConsumerClientId(chainID); consumerRegistered {
         oldConsumerAddr := utils.TMCryptoPublicKeyToConsAddr(oldConsumerKey)
         vscID := GetValidatorSetUpdateId()
         AppendConsumerAddrsToPrune(chainID, vscID, oldConsumerAddr)
-    } else {
-        // the validator had no key assigned on this consumer chain
-        oldConsumerKey := validator.TmConsPublicKey()
-    }
-
-    // check whether the validator is valid, i.e., its power is positive
-    if currentPower := stakingKeeper.GetLastValidatorPower(providerAddr); currentPower > 0 {
-        // to enable multiple calls of AssignConsumerKey in the same block by the same validator
-        // the key assignment replacement should not be overwritten
-        if _, found := GetKeyAssignmentReplacement(chainID, providerConsAddr); !found {
-            // store old key and power for modifying the valset update in EndBlock
-            oldKeyAssignment := abci.ValidatorUpdate{PubKey: oldConsumerKey, Power: currentPower}
-            SetKeyAssignmentReplacement(chainID, providerConsAddr, oldKeyAssignment)
-        }
     }
 } else {
     // if the consumer chain is not registered, then remove the previous reverse mapping
@@ -129,89 +112,24 @@ func (k Keeper) MakeConsumerGenesis(chainID string) (gen consumertypes.GenesisSt
 }
 ```
 
-On `EndBlock` while queueing `VSCPacket`s to send to registered consumer chains:
+Note that key assignment works hand-in-hand with [epochs](https://github.com/cosmos/interchain-security/blob/main/docs/docs/adrs/adr-014-epochs.md).
+For each consumer chain, we store the consumer validator set that is currently (i.e., in this epoch) validating the consumer chain. 
+Specifically, for each validator in the set we store among others, the public key that it is using on the consumer chain during the current (i.e., ongoing) epoch.
+At the end of every epoch, if there were validator set changes on the provider, then for every consumer chain, we construct a `VSCPacket` 
+with all the validator updates and add it to the list of `PendingVSCPacket`s. We compute the validator updates needed by a consumer chain by
+comparing the stored list of consumer validators with the current bonded validators on the provider, with something similar to this:
 ```golang
-func QueueVSCPackets() {
-	valUpdateID := GetValidatorSetUpdateId()
-	// get the validator updates from the staking module
-	valUpdates := stakingKeeper.GetValidatorUpdates()
-
-	IterateConsumerChains(func(chainID, clientID string) (stop bool) {
-		// apply the key assignment to the validator updates
-		valUpdates := ApplyKeyAssignmentToValUpdates(chainID, valUpdates)
-        // ..
-    })
-    // ...
-}
-
-func ApplyKeyAssignmentToValUpdates(
-    chainID string, 
-    valUpdates []abci.ValidatorUpdate,
-) (newUpdates []abci.ValidatorUpdate) {
-    for _, valUpdate := range valUpdates {
-        providerAddr := utils.TMCryptoPublicKeyToConsAddr(valUpdate.PubKey)
-
-        // if a key assignment replacement is found, then
-        // remove the valupdate with the old consumer key
-        // and create two new valupdates
-        prevConsumerKey, _, found := GetKeyAssignmentReplacement(chainID, providerAddr)
-        if found {
-            // set the old consumer key's power to 0
-            newUpdates = append(newUpdates, abci.ValidatorUpdate{
-				PubKey: prevConsumerKey,
-				Power:  0,
-			})
-		    // set the new consumer key's power to the power in the update
-            newConsumerKey := GetValidatorConsumerPubKey(chainID, providerAddr)
-			newUpdates = append(newUpdates, abci.ValidatorUpdate{
-				PubKey: newConsumerKey,
-				Power:  valUpdate.Power,
-			})
-            // delete key assignment replacement
-			DeleteKeyAssignmentReplacement(chainID, providerAddr)
-        } else {
-            // there is no key assignment replacement;
-            // check if the validator's key is assigned
-            consumerKey, found := k.GetValidatorConsumerPubKey(ctx, chainID, providerAddr)
-			if found {
-                // replace the update containing the provider key 
-                // with an update containing the consumer key
-				newUpdates = append(newUpdates, abci.ValidatorUpdate{
-					PubKey: consumerKey,
-					Power:  valUpdate.Power,
-				})
-			} else {
-				// keep the same update
-				newUpdates = append(newUpdates, valUpdate)
-			}
-        }
-    }
-
-    // iterate over the remaining key assignment replacements
-    IterateKeyAssignmentReplacements(chainID, func(
-		pAddr sdk.ConsAddress,
-		prevCKey tmprotocrypto.PublicKey,
-		power int64,
-	) (stop bool) {
-       // set the old consumer key's power to 0
-		newUpdates = append(newUpdates, abci.ValidatorUpdate{
-			PubKey: prevCKey,
-			Power:  0,
-		})
-        // set the new consumer key's power to the power in key assignment replacement
-		newConsumerKey := GetValidatorConsumerPubKey(chainID, pAddr)
-		newUpdates = append(newUpdates, abci.ValidatorUpdate{
-			PubKey: newConsumerKey,
-			Power:  power,
-		})
-		return false
-	})
-
-    // remove all the key assignment replacements
-
-    return newUpdates
-}
+// get the valset that has been validating the consumer chain during this epoch 
+currentValidators := GetConsumerValSet(consumerChain)
+// generate the validator updates needed to be sent through a `VSCPacket` by comparing the current validators 
+// in the epoch with the latest bonded validators
+valUpdates := DiffValidators(currentValidators, stakingmodule.GetBondedValidators())
+// update the current validators set for the upcoming epoch to be the latest bonded validators instead
+SetConsumerValSet(stakingmodule.GetBondedValidators())
 ```
+where `DiffValidators` internally checks if the consumer public key for a validator has changed since the last
+epoch and if so generates a validator update. This way, a validator can change its consumer public key for a consumer
+chain an arbitrary amount of times and only the last set consumer public key would be taken into account.
 
 On receiving a `SlashPacket` from a consumer chain with id `chainID` for a infraction of a validator `data.Validator`:
 ```golang

diff --git a/docs/docs/adrs/adr-014-epochs.md b/docs/docs/adrs/adr-014-epochs.md
@@ -0,0 +1,77 @@
+---
+sidebar_position: 15
+title: Epochs
+---
+# ADR 014: Epochs
+
+## Changelog
+* 2024-01-05: Proposed, first draft of ADR.
+* 2024-02-29: Updated so that it describes the implementation where we store the whole consumer validator set.
+
+## Status
+
+Proposed
+
+## Context
+
+In every block that the provider valset changes, a `VSCPacket` must be sent to every consumer and a corresponding `VSCMaturedPacket` sent back.
+Given that the validator powers may change very often on the provider chain (e.g., the Cosmos Hub), this approach results in a large workload for the relayers. 
+Although the validator powers may change very often, these changes are usually small and have an insignificant impact on the chain's security.
+In other words, the valset on the consumers can be slightly outdated without affecting security. 
+As a matter of fact, this already happens due to relaying delays. 
+
+As a solution, this ADR introduces the concept of _epochs_. 
+An epoch consists of multiple blocks. 
+The provider sends `VSCPacket`s once per epoch. 
+A `VSCPacket` contains all the validator updates that are needed by a consumer chain.
+
+## Decision
+
+The implementation of epochs requires the following changes:
+
+- For each consumer chain, we store the consumer validator set that is currently (i.e., in this epoch) validating the 
+  consumer chain. For each validator in the set we store i) its voting power, and ii) the public key that it is 
+  using on the consumer chain during the current (i.e., ongoing) epoch.
+  The initial consumer validator set for a chain is set during the creation of the consumer genesis.  
+- We introduce the `BlocksPerEpoch` param that sets the number of blocks in an epoch. By default, `BlocksPerEpoch` is
+  set to be 600 which corresponds to 1 hour, assuming 6 seconds per block. This param can be changed through
+  a _governance proposal_. In the provider `EndBlock` we check `BlockHeight() % BlocksPerEpoch() == 0`
+  to decide when an epoch has ended.
+- At the end of every epoch, if there were validator set changes on the provider, then for every consumer chain, we 
+  construct a `VSCPacket` with all the validator updates and add it to the list of `PendingVSCPackets`. We compute the
+  validator updates needed by a consumer chain by comparing the stored list of consumer validators with the current
+  bonded validators on the provider, with something similar to this:
+```go
+// get the valset that has been validating the consumer chain during this epoch 
+currentValidators := GetConsumerValSet(consumerChain)
+// generate the validator updates needed to be sent through a `VSCPacket` by comparing the current validators 
+// in the epoch with the latest bonded validators
+valUpdates := DiffValidators(currentValidators, stakingmodule.GetBondedValidators())
+// update the current validators set for the upcoming epoch to be the latest bonded validators instead
+SetConsumerValSet(stakingmodule.GetBondedValidators())
+```
+Note that a validator can change its consumer public key for a specific consumer chain an arbitrary amount of times during
+a block and during an epoch. Then, when we generate the validator updates in `DiffValidators`, we have to check whether
+the current consumer public key (retrieved by calling `GetValidatorConsumerPubKey`) is different from the consumer public
+key the validator was using in the current epoch.
+
+## Consequences
+
+### Positive
+
+- Reduce the cost of relaying.
+- Reduce the amount of IBC packets needed for ICS.
+- Simplifies [key-assignment code](https://github.com/cosmos/interchain-security/blob/main/docs/docs/adrs/adr-001-key-assignment.md) because
+  we only need to check if the `consumer_public_key` has been modified since the last epoch to generate an update. 
+
+### Negative
+
+- Increase the delay in the propagation of validator set changes (but for reasonable epoch lengths on the order of ~hours or less, this is unlikely to be significant).
+
+### Neutral
+
+N/A
+
+## References
+
+* [EPIC](https://github.com/cosmos/interchain-security/issues/1087)
diff --git a/docs/docs/introduction/params.md b/docs/docs/introduction/params.md
@@ -149,3 +149,15 @@ This param would allow provider binaries to panic deterministically in the event
 
 `RetryDelayPeriod` exists on the consumer for **ICS versions >= v3.2.0** (introduced by the implementation of [ADR-008](../adrs/adr-008-throttle-retries.md)) and is the period at which the consumer retries to send a `SlashPacket` that was rejected by the provider.
 
+
+## Epoch Parameters
+
+### BlocksPerEpoch
+`BlocksPerEpoch` exists on the provider for **ICS versions >= 3.3.0** (introduced by the implementation of [ADR-014](../adrs/adr-014-epochs.md))
+and corresponds to the number of blocks that constitute an epoch. This param is set to 600 by default. Assuming we need 6 seconds to
+commit a block, the duration of an epoch corresponds to 1 hour. This means that a `VSCPacket` would be sent to a consumer
+chain once at the end of every epoch, so once every 600 blocks. This parameter can be adjusted via a governance proposal,
+however careful consideration is needed so that `BlocksPerEpoch` is not too large. A large `BlocksPerEpoch` could lead to a delay
+of `VSCPacket`s and hence potentially lead to [unbonding pausing](https://informal.systems/blog/learning-to-live-with-unbonding-pausing).
+For setting `BlocksPerEpoch`, we also need to consider potential slow chain upgrades that could delay the sending of a 
+`VSCPacket`, as well as potential increases in the time it takes to commit a block (e.g., from 6 seconds to 30 seconds).
diff --git a/proto/interchain_security/ccv/provider/v1/provider.proto b/proto/interchain_security/ccv/provider/v1/provider.proto
@@ -188,6 +188,9 @@ message Params {
   // The fee required to be paid to add a reward denom
   cosmos.base.v1beta1.Coin consumer_reward_denom_registration_fee = 9
       [ (gogoproto.nullable) = false ];
+
+  // The number of blocks that comprise an epoch.
+  int64 blocks_per_epoch = 10;
 }
 
 // SlashAcks contains cons addresses of consumer chain validators
@@ -295,3 +298,14 @@ message ConsumerAddrsToPrune {
   uint64 vsc_id = 2;
   AddressList consumer_addrs = 3;
 }
+
+// ConsumerValidator is used to facilitate epoch-based transitions. It contains relevant info for
+// a validator that is expected to validate on a consumer chain during an epoch.
+message ConsumerValidator {
+  // validator's consensus address on the provider chain
+  bytes provider_cons_addr = 1;
+  // voting power the validator has during this epoch
+  int64 power = 2;
+  // public key the validator uses on the consumer chain during this epoch
+  tendermint.crypto.PublicKey consumer_public_key = 3;
+}
diff --git a/tests/e2e/actions.go b/tests/e2e/actions.go
@@ -1256,6 +1256,12 @@ func (tr TestConfig) relayPacketsGorelayer(
 	action RelayPacketsAction,
 	verbose bool,
 ) {
+	// Because `.app_state.provider.params.blocks_per_epoch` is set to 3 in the E2E tests, we wait 3 blocks
+	// before relaying the packets to guarantee that at least one epoch passes and hence any `VSCPacket`s get
+	// queued and are subsequently relayed.
+	tr.waitBlocks(action.ChainA, 3, 90*time.Second)
+	tr.waitBlocks(action.ChainB, 3, 90*time.Second)
+
 	pathName := tr.GetPathNameForGorelayer(action.ChainA, action.ChainB)
 
 	// rly transact relay-packets [path-name] --channel [channel-id]
@@ -1280,6 +1286,12 @@ func (tr TestConfig) relayPacketsHermes(
 	action RelayPacketsAction,
 	verbose bool,
 ) {
+	// Because `.app_state.provider.params.blocks_per_epoch` is set to 3 in the E2E tests, we wait 3 blocks
+	// before relaying the packets to guarantee that at least one epoch passes and hence any `VSCPacket`s get
+	// queued and are subsequently relayed.
+	tr.waitBlocks(action.ChainA, 3, 90*time.Second)
+	tr.waitBlocks(action.ChainB, 3, 90*time.Second)
+
 	// hermes clear packets ibc0 transfer channel-13
 	//#nosec G204 -- Bypass linter warning for spawning subprocess with cmd arguments.
 	cmd := exec.Command("docker", "exec", tr.containerConfig.InstanceName, "hermes", "clear", "packets",
@@ -1591,7 +1603,7 @@ func (tr TestConfig) invokeDowntimeSlash(action DowntimeSlashAction, verbose boo
 	// Bring validator down
 	tr.setValidatorDowntime(action.Chain, action.Validator, true, verbose)
 	// Wait appropriate amount of blocks for validator to be slashed
-	tr.waitBlocks(action.Chain, 10, 3*time.Minute)
+	tr.waitBlocks(action.Chain, 11, 3*time.Minute)
 	// Bring validator back up
 	tr.setValidatorDowntime(action.Chain, action.Validator, false, verbose)
 }