removed blocks per epoch upper limit

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

@@ -35,8 +35,7 @@ The implementation of epochs requires the following changes:
   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_ to be anywhere between `[1, MaxBlocksPerEpoch]` where `MaxBlocksPerEpoch` can be up to 1200
-  (2 hours if we assume 6 seconds per block). In the provider `EndBlock` we check `BlockHeight() % BlocksPerEpoch() == 0`
+  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

docs/docs/introduction/params.md

@@ -154,5 +154,10 @@ This param would allow provider binaries to panic deterministically in the event
 
 ### 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 and cannot exceed 1200.
-Assuming we need 6 seconds per block, the default value corresponds to 1 hour and the maximum to 2 hours.
+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).

x/ccv/provider/types/params.go

@@ -40,10 +40,6 @@ const (
 	// DefaultBlocksPerEpoch defines the default blocks that constitute an epoch. Assuming we need 6 seconds per block,
 	// an epoch corresponds to 1 hour (6 * 600 = 3600 seconds).
 	DefaultBlocksPerEpoch = 600
-
-	// MaxBlocksPerEpoch defines the maximum blocks that constitute an epoch. Assuming we need 6 seconds per block,
-	// the maximum epoch corresponds to 2 hours (6 * 1200 = 7200 seconds).
-	MaxBlocksPerEpoch = 1200
 )
 
 // Reflection based keys for params subspace
@@ -148,7 +144,7 @@ func (p Params) Validate() error {
 	if err := ValidateCoin(p.ConsumerRewardDenomRegistrationFee); err != nil {
 		return fmt.Errorf("consumer reward denom registration fee is invalid: %s", err)
 	}
-	if err := ValidateBlocksPerEpoch(p.BlocksPerEpoch); err != nil {
+	if err := ccvtypes.ValidateInt64(p.BlocksPerEpoch); err != nil {
 		return fmt.Errorf("blocks per epoch is invalid: %s", err)
 	}
 	return nil
@@ -208,17 +204,3 @@ func ValidateCoin(i interface{}) error {
 
 	return nil
 }
-
-// ValidateBlocksPerEpoch validates the BlocksPerEpoch param is in [1, MaxBlocksPerEpoch]
-func ValidateBlocksPerEpoch(i interface{}) error {
-	if _, ok := i.(int64); !ok {
-		return fmt.Errorf("invalid parameter type: %T", i)
-	}
-	if i.(int64) <= int64(0) {
-		return fmt.Errorf("blocks per epoch must be positive")
-	}
-	if i.(int64) > MaxBlocksPerEpoch {
-		return fmt.Errorf("blocks per epoch have to be at most %d", MaxBlocksPerEpoch)
-	}
-	return nil
-}

x/ccv/provider/types/params_test.go

@@ -57,15 +57,6 @@ func TestValidateParams(t *testing.T) {
 		{"invalid consumer reward denom registration fee amount", types.NewParams(ibctmtypes.NewClientState("", ibctmtypes.DefaultTrustLevel, 0, 0,
 			time.Second*40, clienttypes.Height{}, commitmenttypes.GetSDKSpecs(), []string{"ibc", "upgradedIBCState"}),
 			"0.33", time.Hour, time.Hour, 24*time.Hour, time.Hour, "0.1", sdk.Coin{Denom: "stake", Amount: sdk.NewInt(-10000000)}, 1000), false},
-		{"0 blocks per epoch", types.NewParams(ibctmtypes.NewClientState("", ibctmtypes.DefaultTrustLevel, 0, 0,
-			time.Second*40, clienttypes.Height{}, commitmenttypes.GetSDKSpecs(), []string{"ibc", "upgradedIBCState"}),
-			"0.00", time.Hour, time.Hour, time.Hour, 30*time.Minute, "0.1", sdk.Coin{Denom: "stake", Amount: sdk.NewInt(10000000)}, 0), false},
-		{"exceeding max blocks per epoch", types.NewParams(ibctmtypes.NewClientState("", ibctmtypes.DefaultTrustLevel, 0, 0,
-			time.Second*40, clienttypes.Height{}, commitmenttypes.GetSDKSpecs(), []string{"ibc", "upgradedIBCState"}),
-			"0.00", time.Hour, time.Hour, time.Hour, 30*time.Minute, "0.1", sdk.Coin{Denom: "stake", Amount: sdk.NewInt(10000000)}, types.MaxBlocksPerEpoch+1), false},
-		{"valid blocks per epoch", types.NewParams(ibctmtypes.NewClientState("", ibctmtypes.DefaultTrustLevel, 0, 0,
-			time.Second*40, clienttypes.Height{}, commitmenttypes.GetSDKSpecs(), []string{"ibc", "upgradedIBCState"}),
-			"0.00", time.Hour, time.Hour, time.Hour, 30*time.Minute, "0.1", sdk.Coin{Denom: "stake", Amount: sdk.NewInt(10000000)}, types.MaxBlocksPerEpoch), true},
 	}
 
 	for _, tc := range testCases {