Block Delay Parameter (#171)

* progress on blockdelay

* complete block delay param

* address most comments

* address the rest

* Update modules/light-clients/07-tendermint/types/client_state.go

Co-authored-by: colin axnér <25233464+colin-axner@users.noreply.github.com>

* fix tm tests

* fix lightclient tests

* add connection tests

* DRY consensus metadata setting

* fix genesis and add safety check

* switch param name and default

* set metadata on init and fix tests

* change param name

* CHANGELOG

* add param documentation:

Co-authored-by: colin axnér <25233464+colin-axner@users.noreply.github.com>

CHANGELOG.md

@@ -62,6 +62,7 @@ Ref: https://keepachangelog.com/en/1.0.0/
 * (modules/light-clients/07-tendermint) [\#141](https://github.com/cosmos/ibc-go/pull/141) Freeze the client if there's a conflicting header submitted for an existing consensus state.
 * (modules/core/02-client) [\#8405](https://github.com/cosmos/cosmos-sdk/pull/8405) Refactor IBC client update governance proposals to use a substitute client to update a frozen or expired client.
 * (modules/core/02-client) [\#8673](https://github.com/cosmos/cosmos-sdk/pull/8673) IBC upgrade logic moved to 02-client and an IBC UpgradeProposal is added.
+* (modules/core/03-connection) [\#171](https://github.com/cosmos/ibc-go/pull/171) Introduces a new parameter `MaxExpectedTimePerBlock` to allow connections to calculate and enforce a block delay that is proportional to time delay set by connection.
 
 ### Improvements
 

docs/ibc/proto-docs.md

@@ -155,6 +155,7 @@
     - [ConnectionPaths](#ibc.core.connection.v1.ConnectionPaths)
     - [Counterparty](#ibc.core.connection.v1.Counterparty)
     - [IdentifiedConnection](#ibc.core.connection.v1.IdentifiedConnection)
+    - [Params](#ibc.core.connection.v1.Params)
     - [Version](#ibc.core.connection.v1.Version)
 
     - [State](#ibc.core.connection.v1.State)
@@ -2399,6 +2400,21 @@ identifier field.
 
 
 
+<a name="ibc.core.connection.v1.Params"></a>
+
+### Params
+Params defines the set of Connection parameters.
+
+
+| Field | Type | Label | Description |
+| ----- | ---- | ----- | ----------- |
+| `max_expected_time_per_block` | [uint64](#uint64) |  | maximum expected time per block, used to enforce block delay. This parameter should reflect the largest amount of time that the chain might reasonably take to produce the next block under normal operating conditions. A safe choice is 3-5x the expected time per block. |
+
+
+
+
+
+
 <a name="ibc.core.connection.v1.Version"></a>
 
 ### Version
@@ -2458,6 +2474,7 @@ GenesisState defines the ibc connection submodule's genesis state.
 | `connections` | [IdentifiedConnection](#ibc.core.connection.v1.IdentifiedConnection) | repeated |  |
 | `client_connection_paths` | [ConnectionPaths](#ibc.core.connection.v1.ConnectionPaths) | repeated |  |
 | `next_connection_sequence` | [uint64](#uint64) |  | the sequence for the next generated connection identifier |
+| `params` | [Params](#ibc.core.connection.v1.Params) |  |  |
 
 
 

go.mod

@@ -22,5 +22,4 @@ require (
 	github.com/tendermint/tm-db v0.6.4
 	google.golang.org/genproto v0.0.0-20210114201628-6edceaf6022f
 	google.golang.org/grpc v1.37.0
-	google.golang.org/protobuf v1.26.0
 )

modules/core/02-client/keeper/grpc_query_test.go

@@ -144,7 +144,7 @@ func (suite *KeeperTestSuite) TestQueryClientStates() {
 				expClientStates = types.IdentifiedClientStates{idcs, idcs2}.Sort()
 				req = &types.QueryClientStatesRequest{
 					Pagination: &query.PageRequest{
-						Limit:      7,
+						Limit:      20,
 						CountTotal: true,
 					},
 				}
@@ -159,7 +159,6 @@ func (suite *KeeperTestSuite) TestQueryClientStates() {
 			expClientStates = nil
 
 			tc.malleate()
-
 			// always add localhost which is created by default in init genesis
 			localhostClientState := suite.chainA.GetClientState(exported.Localhost)
 			identifiedLocalhost := types.NewIdentifiedClientState(exported.Localhost, localhostClientState)

modules/core/02-client/keeper/params.go

@@ -5,19 +5,19 @@ import (
 	"github.com/cosmos/ibc-go/modules/core/02-client/types"
 )
 
-// GetAllowedClients retrieves the receive enabled boolean from the paramstore
+// GetAllowedClients retrieves the allowed clients from the paramstore
 func (k Keeper) GetAllowedClients(ctx sdk.Context) []string {
 	var res []string
 	k.paramSpace.Get(ctx, types.KeyAllowedClients, &res)
 	return res
 }
 
-// GetParams returns the total set of ibc-transfer parameters.
+// GetParams returns the total set of ibc-client parameters.
 func (k Keeper) GetParams(ctx sdk.Context) types.Params {
 	return types.NewParams(k.GetAllowedClients(ctx)...)
 }
 
-// SetParams sets the total set of ibc-transfer parameters.
+// SetParams sets the total set of ibc-client parameters.
 func (k Keeper) SetParams(ctx sdk.Context, params types.Params) {
 	k.paramSpace.SetParamSet(ctx, &params)
 }

modules/core/02-client/types/params.go

@@ -4,8 +4,8 @@ import (
 	"fmt"
 	"strings"
 
-	"github.com/cosmos/ibc-go/modules/core/exported"
 	paramtypes "github.com/cosmos/cosmos-sdk/x/params/types"
+	"github.com/cosmos/ibc-go/modules/core/exported"
 )
 
 var (
@@ -21,19 +21,19 @@ func ParamKeyTable() paramtypes.KeyTable {
 	return paramtypes.NewKeyTable().RegisterParamSet(&Params{})
 }
 
-// NewParams creates a new parameter configuration for the ibc transfer module
+// NewParams creates a new parameter configuration for the ibc client module
 func NewParams(allowedClients ...string) Params {
 	return Params{
 		AllowedClients: allowedClients,
 	}
 }
 
-// DefaultParams is the default parameter configuration for the ibc-transfer module
+// DefaultParams is the default parameter configuration for the ibc-client module
 func DefaultParams() Params {
 	return NewParams(DefaultAllowedClients...)
 }
 
-// Validate all ibc-transfer module parameters
+// Validate all ibc-client module parameters
 func (p Params) Validate() error {
 	return validateClients(p.AllowedClients)
 }

modules/core/03-connection/genesis.go

@@ -17,6 +17,7 @@ func InitGenesis(ctx sdk.Context, k keeper.Keeper, gs types.GenesisState) {
 		k.SetClientConnectionPaths(ctx, connPaths.ClientId, connPaths.Paths)
 	}
 	k.SetNextConnectionSequence(ctx, gs.NextConnectionSequence)
+	k.SetParams(ctx, gs.Params)
 }
 
 // ExportGenesis returns the ibc connection submodule's exported genesis.

modules/core/03-connection/keeper/keeper.go

@@ -6,6 +6,7 @@ import (
 	"github.com/cosmos/cosmos-sdk/codec"
 	sdk "github.com/cosmos/cosmos-sdk/types"
 	sdkerrors "github.com/cosmos/cosmos-sdk/types/errors"
+	paramtypes "github.com/cosmos/cosmos-sdk/x/params/types"
 	clienttypes "github.com/cosmos/ibc-go/modules/core/02-client/types"
 	"github.com/cosmos/ibc-go/modules/core/03-connection/types"
 	commitmenttypes "github.com/cosmos/ibc-go/modules/core/23-commitment/types"
@@ -19,15 +20,22 @@ type Keeper struct {
 	types.QueryServer
 
 	storeKey     sdk.StoreKey
+	paramSpace   paramtypes.Subspace
 	cdc          codec.BinaryCodec
 	clientKeeper types.ClientKeeper
 }
 
 // NewKeeper creates a new IBC connection Keeper instance
-func NewKeeper(cdc codec.BinaryCodec, key sdk.StoreKey, ck types.ClientKeeper) Keeper {
+func NewKeeper(cdc codec.BinaryCodec, key sdk.StoreKey, paramSpace paramtypes.Subspace, ck types.ClientKeeper) Keeper {
+	// set KeyTable if it has not already been set
+	if !paramSpace.HasKeyTable() {
+		paramSpace = paramSpace.WithKeyTable(types.ParamKeyTable())
+	}
+
 	return Keeper{
 		storeKey:     key,
 		cdc:          cdc,
+		paramSpace:   paramSpace,
 		clientKeeper: ck,
 	}
 }

modules/core/03-connection/keeper/params.go

@@ -0,0 +1,23 @@
+package keeper
+
+import (
+	sdk "github.com/cosmos/cosmos-sdk/types"
+	"github.com/cosmos/ibc-go/modules/core/03-connection/types"
+)
+
+// GetMaxExpectedTimePerBlock retrieves the maximum expected time per block from the paramstore
+func (k Keeper) GetMaxExpectedTimePerBlock(ctx sdk.Context) uint64 {
+	var res uint64
+	k.paramSpace.Get(ctx, types.KeyMaxExpectedTimePerBlock, &res)
+	return res
+}
+
+// GetParams returns the total set of ibc-connection parameters.
+func (k Keeper) GetParams(ctx sdk.Context) types.Params {
+	return types.NewParams(k.GetMaxExpectedTimePerBlock(ctx))
+}
+
+// SetParams sets the total set of ibc-connection parameters.
+func (k Keeper) SetParams(ctx sdk.Context, params types.Params) {
+	k.paramSpace.SetParamSet(ctx, &params)
+}

modules/core/03-connection/keeper/params_test.go

@@ -0,0 +1,17 @@
+package keeper_test
+
+import (
+	"github.com/cosmos/ibc-go/modules/core/03-connection/types"
+)
+
+func (suite *KeeperTestSuite) TestParams() {
+	expParams := types.DefaultParams()
+
+	params := suite.chainA.App.GetIBCKeeper().ConnectionKeeper.GetParams(suite.chainA.GetContext())
+	suite.Require().Equal(expParams, params)
+
+	expParams.MaxExpectedTimePerBlock = 10
+	suite.chainA.App.GetIBCKeeper().ConnectionKeeper.SetParams(suite.chainA.GetContext(), expParams)
+	params = suite.chainA.App.GetIBCKeeper().ConnectionKeeper.GetParams(suite.chainA.GetContext())
+	suite.Require().Equal(uint64(10), expParams.MaxExpectedTimePerBlock)
+}

modules/core/03-connection/keeper/verify.go

@@ -1,6 +1,8 @@
 package keeper
 
 import (
+	"math"
+
 	sdk "github.com/cosmos/cosmos-sdk/types"
 	sdkerrors "github.com/cosmos/cosmos-sdk/types/errors"
 	clienttypes "github.com/cosmos/ibc-go/modules/core/02-client/types"
@@ -159,9 +161,13 @@ func (k Keeper) VerifyPacketCommitment(
 		return sdkerrors.Wrapf(clienttypes.ErrClientNotActive, "client (%s) status is %s", clientID, status)
 	}
 
+	// get time and block delays
+	timeDelay := connection.GetDelayPeriod()
+	blockDelay := k.getBlockDelay(ctx, connection)
+
 	if err := clientState.VerifyPacketCommitment(
-		clientStore, k.cdc, height,
-		uint64(ctx.BlockTime().UnixNano()), connection.GetDelayPeriod(),
+		ctx, clientStore, k.cdc, height,
+		timeDelay, blockDelay,
 		connection.GetCounterparty().GetPrefix(), proof, portID, channelID,
 		sequence, commitmentBytes,
 	); err != nil {
@@ -195,9 +201,13 @@ func (k Keeper) VerifyPacketAcknowledgement(
 		return sdkerrors.Wrapf(clienttypes.ErrClientNotActive, "client (%s) status is %s", clientID, status)
 	}
 
+	// get time and block delays
+	timeDelay := connection.GetDelayPeriod()
+	blockDelay := k.getBlockDelay(ctx, connection)
+
 	if err := clientState.VerifyPacketAcknowledgement(
-		clientStore, k.cdc, height,
-		uint64(ctx.BlockTime().UnixNano()), connection.GetDelayPeriod(),
+		ctx, clientStore, k.cdc, height,
+		timeDelay, blockDelay,
 		connection.GetCounterparty().GetPrefix(), proof, portID, channelID,
 		sequence, acknowledgement,
 	); err != nil {
@@ -231,9 +241,13 @@ func (k Keeper) VerifyPacketReceiptAbsence(
 		return sdkerrors.Wrapf(clienttypes.ErrClientNotActive, "client (%s) status is %s", clientID, status)
 	}
 
+	// get time and block delays
+	timeDelay := connection.GetDelayPeriod()
+	blockDelay := k.getBlockDelay(ctx, connection)
+
 	if err := clientState.VerifyPacketReceiptAbsence(
-		clientStore, k.cdc, height,
-		uint64(ctx.BlockTime().UnixNano()), connection.GetDelayPeriod(),
+		ctx, clientStore, k.cdc, height,
+		timeDelay, blockDelay,
 		connection.GetCounterparty().GetPrefix(), proof, portID, channelID,
 		sequence,
 	); err != nil {
@@ -266,9 +280,13 @@ func (k Keeper) VerifyNextSequenceRecv(
 		return sdkerrors.Wrapf(clienttypes.ErrClientNotActive, "client (%s) status is %s", clientID, status)
 	}
 
+	// get time and block delays
+	timeDelay := connection.GetDelayPeriod()
+	blockDelay := k.getBlockDelay(ctx, connection)
+
 	if err := clientState.VerifyNextSequenceRecv(
-		clientStore, k.cdc, height,
-		uint64(ctx.BlockTime().UnixNano()), connection.GetDelayPeriod(),
+		ctx, clientStore, k.cdc, height,
+		timeDelay, blockDelay,
 		connection.GetCounterparty().GetPrefix(), proof, portID, channelID,
 		nextSequenceRecv,
 	); err != nil {
@@ -277,3 +295,18 @@ func (k Keeper) VerifyNextSequenceRecv(
 
 	return nil
 }
+
+// getBlockDelay calculates the block delay period from the time delay of the connection
+// and the maximum expected time per block.
+func (k Keeper) getBlockDelay(ctx sdk.Context, connection exported.ConnectionI) uint64 {
+	// expectedTimePerBlock should never be zero, however if it is then return a 0 blcok delay for safety
+	// as the expectedTimePerBlock parameter was not set.
+	expectedTimePerBlock := k.GetMaxExpectedTimePerBlock(ctx)
+	if expectedTimePerBlock == 0 {
+		return 0
+	}
+	// calculate minimum block delay by dividing time delay period
+	// by the expected time per block. Round up the block delay.
+	timeDelay := connection.GetDelayPeriod()
+	return uint64(math.Ceil(float64(timeDelay) / float64(expectedTimePerBlock)))
+}