diff --git a/.changelog/unreleased/improvements/2357-init-params.md b/.changelog/unreleased/improvements/2357-init-params.md new file mode 100644 index 0000000000..392e659b49 --- /dev/null +++ b/.changelog/unreleased/improvements/2357-init-params.md @@ -0,0 +1,3 @@ +- `[x/provider]` Add validation for initial height and set + default values for consumer initialization params. + ([\#2357](https://github.com/cosmos/interchain-security/pull/2357)) \ No newline at end of file diff --git a/.changelog/unreleased/state-breaking/2357-init-params.md b/.changelog/unreleased/state-breaking/2357-init-params.md new file mode 100644 index 0000000000..392e659b49 --- /dev/null +++ b/.changelog/unreleased/state-breaking/2357-init-params.md @@ -0,0 +1,3 @@ +- `[x/provider]` Add validation for initial height and set + default values for consumer initialization params. + ([\#2357](https://github.com/cosmos/interchain-security/pull/2357)) \ No newline at end of file diff --git a/Dockerfile b/Dockerfile index c762216205..12ac2212fa 100644 --- a/Dockerfile +++ b/Dockerfile @@ -29,8 +29,8 @@ RUN go mod tidy RUN make install # Get Hermes build -## TODO: import Hermes release from ghcr.io/informalsystems repository when -## a Hermes release contains the patch in +# TODO: import Hermes release from ghcr.io/informalsystems repository when +# a Hermes release contains the patch in # https://github.com/informalsystems/hermes/pull/4182 FROM --platform=linux/amd64 otacrew/hermes-ics:latest AS hermes-builder diff --git a/Dockerfile.combined b/Dockerfile.combined index 47a8fad86a..7bd09917b9 100644 --- a/Dockerfile.combined +++ b/Dockerfile.combined @@ -24,7 +24,10 @@ FROM --platform=linux/amd64 ${PROVIDER_IMAGE} AS provider FROM --platform=linux/amd64 ${CONSUMER_IMAGE} AS consumer # Get Hermes build -FROM --platform=linux/amd64 ghcr.io/informalsystems/hermes:1.10.2 AS hermes-builder +# TODO: import Hermes release from ghcr.io/informalsystems repository when +# a Hermes release contains the patch in +# https://github.com/informalsystems/hermes/pull/4182 +FROM --platform=linux/amd64 otacrew/hermes-ics:latest AS hermes-builder # Get GoRelayer diff --git a/tests/e2e/actions.go b/tests/e2e/actions.go index 872d19ce6c..06bc94c6af 100644 --- a/tests/e2e/actions.go +++ b/tests/e2e/actions.go @@ -637,7 +637,7 @@ func (tr Chain) submitConsumerAdditionProposal( DistributionTransmissionChannel: action.DistributionChannel, } - consumerId := tr.CreateConsumer(providerChainCfg.ChainId, consumerChainCfg.ChainId, action.From, Metadata, nil, nil) + consumerId := tr.CreateConsumer(providerChainCfg.ChainId, consumerChainCfg.ChainId, action.From, Metadata, &initializationParameters, nil) authority := "cosmos10d07y265gmmuvt4z0w9aw880jnsr700j6zn9kn" // Set the new created consumer-id on the chain's config consumerChainCfg.ConsumerId = consumerId diff --git a/testutil/keeper/unit_test_helpers.go b/testutil/keeper/unit_test_helpers.go index 9c967a739e..929ce7be30 100644 --- a/testutil/keeper/unit_test_helpers.go +++ b/testutil/keeper/unit_test_helpers.go @@ -223,7 +223,7 @@ func SetupForDeleteConsumerChain(t *testing.T, ctx sdk.Context, t.Helper() expectations := GetMocksForCreateConsumerClient(ctx, &mocks, - "chainID", clienttypes.NewHeight(4, 5)) + "chainID", clienttypes.NewHeight(0, 5)) expectations = append(expectations, GetMocksForSetConsumerChain(ctx, &mocks, "chainID")...) gomock.InOrder(expectations...) @@ -286,7 +286,7 @@ func GetTestConsumerMetadata() providertypes.ConsumerMetadata { func GetTestInitializationParameters() providertypes.ConsumerInitializationParameters { return providertypes.ConsumerInitializationParameters{ - InitialHeight: clienttypes.NewHeight(4, 5), + InitialHeight: clienttypes.NewHeight(0, 5), GenesisHash: []byte("gen_hash"), BinaryHash: []byte("bin_hash"), SpawnTime: time.Now().UTC(), @@ -323,7 +323,7 @@ func GetTestMsgUpdateConsumer() providertypes.MsgUpdateConsumer { func GetTestMsgConsumerAddition() providertypes.MsgConsumerAddition { return providertypes.MsgConsumerAddition{ ChainId: "a ChainId", - InitialHeight: clienttypes.NewHeight(4, 5), + InitialHeight: clienttypes.NewHeight(0, 5), GenesisHash: []byte(base64.StdEncoding.EncodeToString([]byte("gen_hash"))), BinaryHash: []byte(base64.StdEncoding.EncodeToString([]byte("bin_hash"))), SpawnTime: time.Now(), diff --git a/x/ccv/provider/keeper/consumer_lifecycle_test.go b/x/ccv/provider/keeper/consumer_lifecycle_test.go index 6b258ae003..200c2b949f 100644 --- a/x/ccv/provider/keeper/consumer_lifecycle_test.go +++ b/x/ccv/provider/keeper/consumer_lifecycle_test.go @@ -56,7 +56,8 @@ func TestPrepareConsumerForLaunch(t *testing.T) { func TestInitializeConsumer(t *testing.T) { now := time.Now().UTC() - consumerId := "13" + consumerId := CONSUMER_ID + chainId := CONSUMER_CHAIN_ID testCases := []struct { name string @@ -69,6 +70,7 @@ func TestInitializeConsumer(t *testing.T) { spawnTime: now, setup: func(pk *providerkeeper.Keeper, ctx sdk.Context, spawnTime time.Time) { pk.SetConsumerPhase(ctx, consumerId, providertypes.CONSUMER_PHASE_REGISTERED) + pk.SetConsumerChainId(ctx, consumerId, chainId) err := pk.SetConsumerInitializationParameters(ctx, consumerId, providertypes.ConsumerInitializationParameters{ SpawnTime: spawnTime, @@ -89,6 +91,7 @@ func TestInitializeConsumer(t *testing.T) { spawnTime: now, setup: func(pk *providerkeeper.Keeper, ctx sdk.Context, spawnTime time.Time) { pk.SetConsumerPhase(ctx, consumerId, providertypes.CONSUMER_PHASE_LAUNCHED) + pk.SetConsumerChainId(ctx, consumerId, chainId) err := pk.SetConsumerInitializationParameters(ctx, consumerId, providertypes.ConsumerInitializationParameters{ SpawnTime: spawnTime, @@ -110,6 +113,7 @@ func TestInitializeConsumer(t *testing.T) { spawnTime: now, setup: func(pk *providerkeeper.Keeper, ctx sdk.Context, spawnTime time.Time) { pk.SetConsumerPhase(ctx, consumerId, providertypes.CONSUMER_PHASE_REGISTERED) + pk.SetConsumerChainId(ctx, consumerId, chainId) err := pk.SetConsumerInitializationParameters(ctx, consumerId, providertypes.ConsumerInitializationParameters{ SpawnTime: time.Time{}, @@ -150,7 +154,7 @@ func TestBeginBlockLaunchConsumers(t *testing.T) { initializationParameters := []providertypes.ConsumerInitializationParameters{ { - InitialHeight: clienttypes.NewHeight(3, 4), + InitialHeight: clienttypes.NewHeight(0, 4), GenesisHash: []byte{}, BinaryHash: []byte{}, SpawnTime: now.Add(-time.Hour * 2).UTC(), @@ -163,7 +167,7 @@ func TestBeginBlockLaunchConsumers(t *testing.T) { DistributionTransmissionChannel: "", }, { - InitialHeight: clienttypes.NewHeight(3, 4), + InitialHeight: clienttypes.NewHeight(0, 4), GenesisHash: []byte{}, BinaryHash: []byte{}, SpawnTime: now.Add(-time.Hour).UTC(), @@ -176,7 +180,7 @@ func TestBeginBlockLaunchConsumers(t *testing.T) { DistributionTransmissionChannel: "", }, { - InitialHeight: clienttypes.NewHeight(3, 4), + InitialHeight: clienttypes.NewHeight(0, 4), GenesisHash: []byte{}, BinaryHash: []byte{}, SpawnTime: now.Add(time.Hour).UTC(), @@ -189,7 +193,7 @@ func TestBeginBlockLaunchConsumers(t *testing.T) { DistributionTransmissionChannel: "", }, { - InitialHeight: clienttypes.NewHeight(3, 4), + InitialHeight: clienttypes.NewHeight(0, 4), GenesisHash: []byte{}, BinaryHash: []byte{}, SpawnTime: now.Add(-time.Hour).UTC(), @@ -202,7 +206,7 @@ func TestBeginBlockLaunchConsumers(t *testing.T) { DistributionTransmissionChannel: "", }, { - InitialHeight: clienttypes.NewHeight(3, 4), + InitialHeight: clienttypes.NewHeight(0, 4), GenesisHash: []byte{}, BinaryHash: []byte{}, SpawnTime: now.Add(-time.Minute).UTC(), @@ -283,11 +287,11 @@ func TestBeginBlockLaunchConsumers(t *testing.T) { // Expect genesis and client creation for only the first, second, and fifth chains (spawn time already passed and valid) expectedCalls := testkeeper.GetMocksForMakeConsumerGenesis(ctx, &mocks, time.Hour) - expectedCalls = append(expectedCalls, testkeeper.GetMocksForCreateConsumerClient(ctx, &mocks, "chain0", clienttypes.NewHeight(3, 4))...) + expectedCalls = append(expectedCalls, testkeeper.GetMocksForCreateConsumerClient(ctx, &mocks, "chain0", clienttypes.NewHeight(0, 4))...) expectedCalls = append(expectedCalls, testkeeper.GetMocksForMakeConsumerGenesis(ctx, &mocks, time.Hour)...) - expectedCalls = append(expectedCalls, testkeeper.GetMocksForCreateConsumerClient(ctx, &mocks, "chain1", clienttypes.NewHeight(3, 4))...) + expectedCalls = append(expectedCalls, testkeeper.GetMocksForCreateConsumerClient(ctx, &mocks, "chain1", clienttypes.NewHeight(0, 4))...) expectedCalls = append(expectedCalls, testkeeper.GetMocksForMakeConsumerGenesis(ctx, &mocks, time.Hour)...) - expectedCalls = append(expectedCalls, testkeeper.GetMocksForCreateConsumerClient(ctx, &mocks, "chain3", clienttypes.NewHeight(3, 4))...) + expectedCalls = append(expectedCalls, testkeeper.GetMocksForCreateConsumerClient(ctx, &mocks, "chain3", clienttypes.NewHeight(0, 4))...) gomock.InOrder(expectedCalls...) @@ -479,7 +483,7 @@ func TestCreateConsumerClient(t *testing.T) { // Valid client creation is asserted with mock expectations here gomock.InOrder( - testkeeper.GetMocksForCreateConsumerClient(ctx, mocks, CONSUMER_CHAIN_ID, clienttypes.NewHeight(4, 5))..., + testkeeper.GetMocksForCreateConsumerClient(ctx, mocks, CONSUMER_CHAIN_ID, clienttypes.NewHeight(0, 5))..., ) }, expClientCreated: true, @@ -774,7 +778,7 @@ func TestBeginBlockStopConsumers(t *testing.T) { chainId := chainIds[i] // A consumer chain is setup corresponding to each consumerId, making these mocks necessary expectations = append(expectations, testkeeper.GetMocksForCreateConsumerClient(ctx, &mocks, - chainId, clienttypes.NewHeight(2, 3))...) + chainId, clienttypes.NewHeight(0, 3))...) expectations = append(expectations, testkeeper.GetMocksForSetConsumerChain(ctx, &mocks, chainId)...) } // Only first two consumer chains should be stopped @@ -789,7 +793,7 @@ func TestBeginBlockStopConsumers(t *testing.T) { for i, consumerId := range consumerIds { // Setup a valid consumer chain for each consumerId initializationRecord := testkeeper.GetTestInitializationParameters() - initializationRecord.InitialHeight = clienttypes.NewHeight(2, 3) + initializationRecord.InitialHeight = clienttypes.NewHeight(0, 3) registrationRecord := testkeeper.GetTestConsumerMetadata() providerKeeper.SetConsumerChainId(ctx, consumerId, chainIds[i]) diff --git a/x/ccv/provider/keeper/grpc_query_test.go b/x/ccv/provider/keeper/grpc_query_test.go index 28658d7970..a8fe617f53 100644 --- a/x/ccv/provider/keeper/grpc_query_test.go +++ b/x/ccv/provider/keeper/grpc_query_test.go @@ -312,11 +312,15 @@ func TestQueryConsumerChainsValidatorHasToValidate(t *testing.T) { // set up some consumer chains for i := 0; i < consumerNum; i++ { - chainID := "consumer-" + strconv.Itoa(i) + revisionNumber := i + 1 + chainID := "consumer-" + strconv.Itoa(revisionNumber) metadata := types.ConsumerMetadata{Name: chainID} + initializationParameters := types.DefaultConsumerInitializationParameters() + initializationParameters.InitialHeight.RevisionNumber = uint64(revisionNumber) msg := types.MsgCreateConsumer{ - ChainId: chainID, - Metadata: metadata, + ChainId: chainID, + Metadata: metadata, + InitializationParameters: &initializationParameters, } resp, err := msgServer.CreateConsumer(ctx, &msg) require.NoError(t, err) @@ -535,7 +539,7 @@ func TestQueryConsumerChain(t *testing.T) { defer ctrl.Finish() consumerId := "0" - chainId := "consumer-1" + chainId := "consumer" req := types.QueryConsumerChainRequest{ ConsumerId: consumerId, @@ -644,11 +648,15 @@ func TestQueryConsumerChains(t *testing.T) { // create four consumer chains in different phase for i := 0; i < consumerNum; i++ { - chainID := "consumer-" + strconv.Itoa(i) + revisionNumber := i + 1 + chainID := "consumer-" + strconv.Itoa(revisionNumber) metadata := types.ConsumerMetadata{Name: chainID} + initializationParameters := types.DefaultConsumerInitializationParameters() + initializationParameters.InitialHeight.RevisionNumber = uint64(revisionNumber) msg := types.MsgCreateConsumer{ - ChainId: chainID, - Metadata: metadata, + ChainId: chainID, + Metadata: metadata, + InitializationParameters: &initializationParameters, } resp, err := msgServer.CreateConsumer(ctx, &msg) require.NoError(t, err) diff --git a/x/ccv/provider/keeper/msg_server.go b/x/ccv/provider/keeper/msg_server.go index a66dfd9135..89970c4b70 100644 --- a/x/ccv/provider/keeper/msg_server.go +++ b/x/ccv/provider/keeper/msg_server.go @@ -378,7 +378,7 @@ func (k msgServer) CreateConsumer(goCtx context.Context, msg *types.MsgCreateCon // initialization parameters are optional and hence could be nil; // in that case, set the default - initializationParameters := types.ConsumerInitializationParameters{} // default params + initializationParameters := types.DefaultConsumerInitializationParameters() // default params if msg.InitializationParameters != nil { initializationParameters = *msg.InitializationParameters } diff --git a/x/ccv/provider/keeper/msg_server_test.go b/x/ccv/provider/keeper/msg_server_test.go index d90fde30ce..7100f44079 100644 --- a/x/ccv/provider/keeper/msg_server_test.go +++ b/x/ccv/provider/keeper/msg_server_test.go @@ -84,7 +84,7 @@ func TestUpdateConsumer(t *testing.T) { // create a chain before updating it createConsumerResponse, err := msgServer.CreateConsumer(ctx, &providertypes.MsgCreateConsumer{ - Submitter: "submitter", ChainId: "chainId", + Submitter: "submitter", ChainId: "chainId-1", Metadata: providertypes.ConsumerMetadata{ Name: "name", Description: "description", @@ -113,6 +113,7 @@ func TestUpdateConsumer(t *testing.T) { } expectedInitializationParameters := testkeeper.GetTestInitializationParameters() + expectedInitializationParameters.InitialHeight.RevisionNumber = 1 expectedPowerShapingParameters := testkeeper.GetTestPowerShapingParameters() expectedOwnerAddress := "cosmos1dkas8mu4kyhl5jrh4nzvm65qz588hy9qcz08la" diff --git a/x/ccv/provider/keeper/permissionless.go b/x/ccv/provider/keeper/permissionless.go index 43504a483e..dc96a5a062 100644 --- a/x/ccv/provider/keeper/permissionless.go +++ b/x/ccv/provider/keeper/permissionless.go @@ -134,6 +134,14 @@ func (k Keeper) SetConsumerInitializationParameters(ctx sdk.Context, consumerId if err != nil { return fmt.Errorf("failed to marshal initialization parameters (%+v) for consumer id (%s): %w", parameters, consumerId, err) } + chainId, err := k.GetConsumerChainId(ctx, consumerId) + if err != nil { + return fmt.Errorf("failed to get consumer chain ID for consumer id (%s): %w", consumerId, err) + } + // validate that the initial height matches the chain ID + if err := types.ValidateInitialHeight(parameters.InitialHeight, chainId); err != nil { + return fmt.Errorf("invalid initial height for consumer id (%s): %w", consumerId, err) + } store.Set(types.ConsumerIdToInitializationParametersKey(consumerId), bz) return nil } diff --git a/x/ccv/provider/keeper/permissionless_test.go b/x/ccv/provider/keeper/permissionless_test.go index 145afb43f1..3039d7b282 100644 --- a/x/ccv/provider/keeper/permissionless_test.go +++ b/x/ccv/provider/keeper/permissionless_test.go @@ -150,14 +150,16 @@ func TestConsumerInitializationParameters(t *testing.T) { HistoricalEntries: 456, DistributionTransmissionChannel: "distribution_transmission_channel", } - providerKeeper.SetConsumerInitializationParameters(ctx, CONSUMER_ID, expectedInitializationParameters) + providerKeeper.SetConsumerChainId(ctx, CONSUMER_ID, "chain-1") + err = providerKeeper.SetConsumerInitializationParameters(ctx, CONSUMER_ID, expectedInitializationParameters) + require.NoError(t, err) actualInitializationParameters, err := providerKeeper.GetConsumerInitializationParameters(ctx, CONSUMER_ID) require.NoError(t, err) require.Equal(t, expectedInitializationParameters, actualInitializationParameters) // assert that overwriting the current initialization record works expectedInitializationParameters = providertypes.ConsumerInitializationParameters{ - InitialHeight: types.Height{RevisionNumber: 2, RevisionHeight: 3}, + InitialHeight: types.Height{RevisionNumber: 1, RevisionHeight: 3}, GenesisHash: []byte{2, 3}, BinaryHash: []byte{4, 5}, SpawnTime: time.Unix(2, 3).UTC(), diff --git a/x/ccv/provider/types/msg.go b/x/ccv/provider/types/msg.go index 05fa7c6ee1..acfdf1bd8b 100644 --- a/x/ccv/provider/types/msg.go +++ b/x/ccv/provider/types/msg.go @@ -3,9 +3,11 @@ package types import ( "encoding/json" "fmt" - "github.com/cosmos/ibc-go/v8/modules/apps/transfer/types" "strings" + "github.com/cosmos/ibc-go/v8/modules/apps/transfer/types" + + clienttypes "github.com/cosmos/ibc-go/v8/modules/core/02-client/types" ibctmtypes "github.com/cosmos/ibc-go/v8/modules/light-clients/07-tendermint" errorsmod "cosmossdk.io/errors" @@ -620,3 +622,11 @@ func validateProviderAddress(addr, signer string) error { return nil } + +func ValidateInitialHeight(initialHeight clienttypes.Height, chainID string) error { + revision := clienttypes.ParseChainID(chainID) + if initialHeight.RevisionNumber != revision { + return fmt.Errorf("chain ID (%s) doesn't match revision number (%d)", chainID, initialHeight.RevisionNumber) + } + return nil +} diff --git a/x/ccv/provider/types/msg_test.go b/x/ccv/provider/types/msg_test.go index 84b40c043e..b0f0f11102 100644 --- a/x/ccv/provider/types/msg_test.go +++ b/x/ccv/provider/types/msg_test.go @@ -647,3 +647,66 @@ func TestMsgAssignConsumerKeyValidateBasic(t *testing.T) { }) } } + +func TestValidateInitialHeight(t *testing.T) { + testCases := []struct { + name string + chainId string + initialHeight clienttypes.Height + expPass bool + }{ + { + name: "valid with revision number", + chainId: "chain-1", + initialHeight: clienttypes.Height{ + RevisionNumber: 1, + RevisionHeight: 0, + }, + expPass: true, + }, + { + name: "valid without revision number", + chainId: "chain", + initialHeight: clienttypes.Height{ + RevisionNumber: 0, + RevisionHeight: 0, + }, + expPass: true, + }, + { + name: "invalid without revision number", + chainId: "chain", + initialHeight: clienttypes.Height{ + RevisionNumber: 1, + RevisionHeight: 0, + }, + expPass: false, + }, + { + name: "invalid without revision number", + chainId: "chain-1", + initialHeight: clienttypes.Height{ + RevisionNumber: 0, + RevisionHeight: 0, + }, + expPass: false, + }, + { + name: "valid: evmos-like chain IDs", + chainId: "evmos_9001-2", + initialHeight: clienttypes.Height{ + RevisionNumber: 2, + RevisionHeight: 0, + }, + expPass: true, + }, + } + for _, tc := range testCases { + err := types.ValidateInitialHeight(tc.initialHeight, tc.chainId) + if tc.expPass { + require.NoError(t, err, "valid case: '%s' should not return error. got %w", tc.name, err) + } else { + require.Error(t, err, "invalid case: '%s' must return error but got none", tc.name) + } + } +} diff --git a/x/ccv/provider/types/provider.go b/x/ccv/provider/types/provider.go new file mode 100644 index 0000000000..9fd3e32718 --- /dev/null +++ b/x/ccv/provider/types/provider.go @@ -0,0 +1,28 @@ +package types + +import ( + "time" + + clienttypes "github.com/cosmos/ibc-go/v8/modules/core/02-client/types" + + ccv "github.com/cosmos/interchain-security/v6/x/ccv/types" +) + +func DefaultConsumerInitializationParameters() ConsumerInitializationParameters { + return ConsumerInitializationParameters{ + InitialHeight: clienttypes.Height{ + RevisionNumber: 1, + RevisionHeight: 1, + }, + GenesisHash: []byte{}, + BinaryHash: []byte{}, + SpawnTime: time.Time{}, + UnbondingPeriod: ccv.DefaultConsumerUnbondingPeriod, + CcvTimeoutPeriod: ccv.DefaultCCVTimeoutPeriod, + TransferTimeoutPeriod: ccv.DefaultTransferTimeoutPeriod, + ConsumerRedistributionFraction: ccv.DefaultConsumerRedistributeFrac, + BlocksPerDistributionTransmission: ccv.DefaultBlocksPerDistributionTransmission, + HistoricalEntries: ccv.DefaultHistoricalEntries, + DistributionTransmissionChannel: "", + } +} diff --git a/x/ccv/types/params.go b/x/ccv/types/params.go index 2a29eeb086..03b9d3f979 100644 --- a/x/ccv/types/params.go +++ b/x/ccv/types/params.go @@ -34,10 +34,10 @@ const ( // (and for consistency with other protobuf schemas defined for ccv). DefaultHistoricalEntries = int64(stakingtypes.DefaultHistoricalEntries) - // In general, the default unbonding period on the consumer is one week less + // In general, the default unbonding period on the consumer is one day less // than the default unbonding period on the provider, where the provider uses // the staking module default. - DefaultConsumerUnbondingPeriod = stakingtypes.DefaultUnbondingTime - 7*24*time.Hour + DefaultConsumerUnbondingPeriod = stakingtypes.DefaultUnbondingTime - 24*time.Hour // Default retry delay period is 1 hour. DefaultRetryDelayPeriod = time.Hour