Merge branch 'main' into dependabot/go_modules/main/github.com/golang…

…/protobuf-1.5.4

.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))

.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))

.github/workflows/docker-publish.yml

@@ -81,7 +81,7 @@ jobs:
       # https://github.com/docker/build-push-action
       - name: Build and push Docker image
         id: build-and-push
-        uses: docker/build-push-action@4a13e500e55cf31b7a5d59a38ab2040ab0f42f56 #v5.1.0
+        uses: docker/build-push-action@af5a7ed5ba88268d5278f7203fb52cd833f66d6e #v5.2.0
         with:
           context: .
           file: ./Dockerfile

.github/workflows/proto-registry.yml

@@ -13,7 +13,7 @@ jobs:
     runs-on: ubuntu-latest
     steps:
       - uses: actions/checkout@v4
-      - uses: bufbuild/buf-setup-action@v1.29.0
+      - uses: bufbuild/buf-setup-action@v1.30.0
       - uses: bufbuild/buf-push-action@v1
         with:
           input: "proto"

.github/workflows/proto.yml

@@ -14,7 +14,7 @@ jobs:
     runs-on: ubuntu-latest
     steps:
       - uses: actions/checkout@v4
-      - uses: bufbuild/buf-setup-action@v1.29.0
+      - uses: bufbuild/buf-setup-action@v1.30.0
       - uses: bufbuild/buf-breaking-action@v1
         with:
           input: "proto"

Makefile

@@ -140,10 +140,8 @@ test-trace:
 # Note: this is *not* using the Quint models to test the system,
 # this tests/verifies the Quint models *themselves*.
 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;\
-	quint run --invariant "all{ValidatorUpdatesArePropagatedKeyAssignmentInv,ValidatorSetHasExistedKeyAssignmentInv,SameVscPacketsKeyAssignmentInv,MatureOnTimeInv,EventuallyMatureOnProviderInv,KeyAssignmentRulesInv}" tests/mbt/model/ccv_model.qnt --step stepKeyAssignment --max-steps 200 --max-samples 200
+	cd tests/mbt/model;\
+	../run_invariants.sh
 
 
 

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

docs/docs/adrs/adr-014-epochs.md

@@ -5,7 +5,8 @@ title: Epochs
 # ADR 014: Epochs
 
 ## Changelog
-* 2024-01-105: Proposed, first draft of ADR.
+* 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
 
@@ -22,37 +23,51 @@ 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 valset changes that occurred throughout the epoch. 
+A `VSCPacket` contains all the validator updates that are needed by a consumer chain.
 
 ## Decision
 
 The implementation of epochs requires the following changes:
 
-- Add a param that sets the number of blocks in an epoch, i.e., `BlocksPerEpoch`. 
-  We can use `BlockHeight() % BlocksPerEpoch == 0` to decide when an epoch is over. 
-  Note that `BlocksPerEpoch` can also be a hardcoded constant as it's unlikely that it will change often.
-- In every provider `EndBlock()`, instead of queueing `VSCPacket` data for every consumer chain, we accumulate the validator changes (similarly to how is done on the consumer, see `AccumulateChanges`). 
-- Modify the key assignment logic to allow for `MustApplyKeyAssignmentToValUpdates` to be called once per epoch. 
-  Currently, this method is called in every block before queueing a `VSCPacket`. 
-  Also, the method uses the `KeyAssignmentReplacement` state, which is pruned at the end of every block. 
-  This needs to be done once per epoch instead.
-- At the end of every epoch, if there were validator set changes on the provider, then for every consumer chain, construct a `VSCPacket` with all the accumulated validator changes and add it to the list of `PendingVSCPackets`.
-
-As an optional change, to better accommodate [the Partial Set Security design](https://informalsystems.notion.site/Partial-Set-Security-398ca9a1453740068be5c7964a4059bb), the validator changes should be accumulated per consumer chain. 
-Like this, it would make it easier to have validators opting out from certain consumer chains. 
+- 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
 
-- Additional logic on the provider side as valset changes need to be accumulated. 
-- The changes might impact the key-assignment logic so special care is needed to avoid introducing bugs.
 - 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

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).

go.mod

@@ -29,8 +29,8 @@ require (
 	golang.org/x/net v0.20.0 // indirect
 	golang.org/x/sys v0.16.0 // indirect
 	google.golang.org/genproto v0.0.0-20240123012728-ef4313101c80 // indirect
-	google.golang.org/grpc v1.62.0
 	google.golang.org/protobuf v1.33.0
+	google.golang.org/grpc v1.62.1
 	gopkg.in/yaml.v2 v2.4.0
 )
 

go.sum

@@ -1644,8 +1644,8 @@ google.golang.org/grpc v1.48.0/go.mod h1:vN9eftEi1UMyUsIF80+uQXhHjbXYbm0uXoFCACu
 google.golang.org/grpc v1.49.0/go.mod h1:ZgQEeidpAuNRZ8iRrlBKXZQP1ghovWIVhdJRyCDK+GI=
 google.golang.org/grpc v1.50.0/go.mod h1:ZgQEeidpAuNRZ8iRrlBKXZQP1ghovWIVhdJRyCDK+GI=
 google.golang.org/grpc v1.50.1/go.mod h1:ZgQEeidpAuNRZ8iRrlBKXZQP1ghovWIVhdJRyCDK+GI=
-google.golang.org/grpc v1.62.0 h1:HQKZ/fa1bXkX1oFOvSjmZEUL8wLSaZTjCcLAlmZRtdk=
-google.golang.org/grpc v1.62.0/go.mod h1:IWTG0VlJLCh1SkC58F7np9ka9mx/WNkjl4PGJaiq+QE=
+google.golang.org/grpc v1.62.1 h1:B4n+nfKzOICUXMgyrNd19h/I9oH0L1pizfk1d4zSgTk=
+google.golang.org/grpc v1.62.1/go.mod h1:IWTG0VlJLCh1SkC58F7np9ka9mx/WNkjl4PGJaiq+QE=
 google.golang.org/grpc/cmd/protoc-gen-go-grpc v1.1.0/go.mod h1:6Kw0yEErY5E/yWrBtf03jp27GLLJujG4z/JK95pnjjw=
 google.golang.org/protobuf v0.0.0-20200109180630-ec00e32a8dfd/go.mod h1:DFci5gLYBciE7Vtevhsrf46CRTquxDuWsQurQQe4oz8=
 google.golang.org/protobuf v0.0.0-20200221191635-4d8936d0db64/go.mod h1:kwYJMbMJ01Woi6D6+Kah6886xMZcty6N08ah7+eCXa0=

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;
+}

tests/e2e/actions.go

@@ -1431,6 +1431,12 @@ func (tr TestConfig) relayPacketsGorelayer(
 	target ExecutionTarget,
 	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]
@@ -1455,6 +1461,12 @@ func (tr TestConfig) relayPacketsHermes(
 	target ExecutionTarget,
 	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
 	cmd := target.ExecCommand("hermes", "clear", "packets",
 		"--chain", string(tr.chainConfigs[action.ChainA].ChainId),
@@ -1763,7 +1775,7 @@ func (tr TestConfig) invokeDowntimeSlash(action DowntimeSlashAction, target Exec
 	// Bring validator down
 	tr.setValidatorDowntime(action.Chain, action.Validator, true, target, 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, target, verbose)
 }