From 94b62c157104cf32e898f3823f26c6aa9e2bcb0b Mon Sep 17 00:00:00 2001 From: mpoke Date: Thu, 29 Aug 2024 15:54:05 +0200 Subject: [PATCH 01/13] handle errors --- testutil/ibc_testing/generic_setup.go | 15 ++++--- x/ccv/provider/keeper/permissionless_test.go | 42 +++++++++++++------- x/ccv/provider/keeper/proposal.go | 19 +++++++-- x/ccv/provider/keeper/proposal_test.go | 38 ++++++++++++------ 4 files changed, 79 insertions(+), 35 deletions(-) diff --git a/testutil/ibc_testing/generic_setup.go b/testutil/ibc_testing/generic_setup.go index ca46aff6b4..10a5d6abae 100644 --- a/testutil/ibc_testing/generic_setup.go +++ b/testutil/ibc_testing/generic_setup.go @@ -160,11 +160,15 @@ func AddConsumer[Tp testutil.ProviderApp, Tc testutil.ConsumerApp]( FirstConsumerID = consumerId } providerKeeper.SetConsumerChainId(providerChain.GetContext(), consumerId, chainID) - providerKeeper.SetConsumerMetadata(providerChain.GetContext(), consumerId, consumerMetadata) - providerKeeper.SetConsumerInitializationParameters(providerChain.GetContext(), consumerId, initializationParameters) - providerKeeper.SetConsumerPowerShapingParameters(providerChain.GetContext(), consumerId, powerShapingParameters) + err := providerKeeper.SetConsumerMetadata(providerChain.GetContext(), consumerId, consumerMetadata) + s.Require().NoError(err) + err = providerKeeper.SetConsumerInitializationParameters(providerChain.GetContext(), consumerId, initializationParameters) + s.Require().NoError(err) + err = providerKeeper.SetConsumerPowerShapingParameters(providerChain.GetContext(), consumerId, powerShapingParameters) + s.Require().NoError(err) providerKeeper.SetConsumerPhase(providerChain.GetContext(), consumerId, providertypes.ConsumerPhase_CONSUMER_PHASE_INITIALIZED) - providerKeeper.AppendConsumerToBeLaunched(providerChain.GetContext(), consumerId, coordinator.CurrentTime) + err = providerKeeper.AppendConsumerToBeLaunched(providerChain.GetContext(), consumerId, coordinator.CurrentTime) + s.Require().NoError(err) // opt-in all validators lastVals, err := providerApp.GetProviderKeeper().GetLastBondedValidators(providerChain.GetContext()) @@ -173,7 +177,8 @@ func AddConsumer[Tp testutil.ProviderApp, Tc testutil.ConsumerApp]( for _, v := range lastVals { consAddr, _ := v.GetConsAddr() providerKeeper.SetOptedIn(providerChain.GetContext(), consumerId, providertypes.NewProviderConsAddress(consAddr)) - providerKeeper.AppendOptedInConsumerId(providerChain.GetContext(), providertypes.NewProviderConsAddress(consAddr), consumerId) + err = providerKeeper.AppendOptedInConsumerId(providerChain.GetContext(), providertypes.NewProviderConsAddress(consAddr), consumerId) + s.Require().NoError(err) } // commit the state on the provider chain diff --git a/x/ccv/provider/keeper/permissionless_test.go b/x/ccv/provider/keeper/permissionless_test.go index f4eb4922c1..a797d255ba 100644 --- a/x/ccv/provider/keeper/permissionless_test.go +++ b/x/ccv/provider/keeper/permissionless_test.go @@ -269,17 +269,20 @@ func TestConsumersToBeLaunched(t *testing.T) { defer ctrl.Finish() spawnTime := time.Now() - providerKeeper.AppendConsumerToBeLaunched(ctx, "consumerId1", spawnTime) + err := providerKeeper.AppendConsumerToBeLaunched(ctx, "consumerId1", spawnTime) + require.NoError(t, err) consumers, err := providerKeeper.GetConsumersToBeLaunched(ctx, spawnTime) require.NoError(t, err) require.Equal(t, []string{"consumerId1"}, consumers.Ids) - providerKeeper.AppendConsumerToBeLaunched(ctx, "consumerId2", spawnTime) + err = providerKeeper.AppendConsumerToBeLaunched(ctx, "consumerId2", spawnTime) + require.NoError(t, err) consumers, err = providerKeeper.GetConsumersToBeLaunched(ctx, spawnTime) require.NoError(t, err) require.Equal(t, []string{"consumerId1", "consumerId2"}, consumers.Ids) - providerKeeper.AppendConsumerToBeLaunched(ctx, "consumerId3", spawnTime) + err = providerKeeper.AppendConsumerToBeLaunched(ctx, "consumerId3", spawnTime) + require.NoError(t, err) consumers, err = providerKeeper.GetConsumersToBeLaunched(ctx, spawnTime) require.NoError(t, err) require.Equal(t, []string{"consumerId1", "consumerId2", "consumerId3"}, consumers.Ids) @@ -292,7 +295,8 @@ func TestConsumersToBeLaunched(t *testing.T) { // also add consumer ids under a different spawn time and verify everything under the original spawn time is still there spawnTimePlusOneHour := spawnTime.Add(time.Hour) - providerKeeper.AppendConsumerToBeLaunched(ctx, "consumerId4", spawnTimePlusOneHour) + err = providerKeeper.AppendConsumerToBeLaunched(ctx, "consumerId4", spawnTimePlusOneHour) + require.NoError(t, err) consumers, err = providerKeeper.GetConsumersToBeLaunched(ctx, spawnTimePlusOneHour) require.NoError(t, err) require.Equal(t, []string{"consumerId4"}, consumers.Ids) @@ -331,17 +335,20 @@ func TestConsumersToBeStopped(t *testing.T) { defer ctrl.Finish() stopTime := time.Now() - providerKeeper.AppendConsumerToBeStopped(ctx, "consumerId1", stopTime) + err := providerKeeper.AppendConsumerToBeStopped(ctx, "consumerId1", stopTime) + require.NoError(t, err) consumers, err := providerKeeper.GetConsumersToBeStopped(ctx, stopTime) require.NoError(t, err) require.Equal(t, []string{"consumerId1"}, consumers.Ids) - providerKeeper.AppendConsumerToBeStopped(ctx, "consumerId2", stopTime) + err = providerKeeper.AppendConsumerToBeStopped(ctx, "consumerId2", stopTime) + require.NoError(t, err) consumers, err = providerKeeper.GetConsumersToBeStopped(ctx, stopTime) require.NoError(t, err) require.Equal(t, []string{"consumerId1", "consumerId2"}, consumers.Ids) - providerKeeper.AppendConsumerToBeStopped(ctx, "consumerId3", stopTime) + err = providerKeeper.AppendConsumerToBeStopped(ctx, "consumerId3", stopTime) + require.NoError(t, err) consumers, err = providerKeeper.GetConsumersToBeStopped(ctx, stopTime) require.NoError(t, err) require.Equal(t, []string{"consumerId1", "consumerId2", "consumerId3"}, consumers.Ids) @@ -354,7 +361,8 @@ func TestConsumersToBeStopped(t *testing.T) { // also add consumer ids under a different stop time and verify everything under the original stop time is still there stopTimePlusOneHour := stopTime.Add(time.Hour) - providerKeeper.AppendConsumerToBeStopped(ctx, "consumerId4", stopTimePlusOneHour) + err = providerKeeper.AppendConsumerToBeStopped(ctx, "consumerId4", stopTimePlusOneHour) + require.NoError(t, err) consumers, err = providerKeeper.GetConsumersToBeStopped(ctx, stopTimePlusOneHour) require.NoError(t, err) require.Equal(t, []string{"consumerId4"}, consumers.Ids) @@ -474,9 +482,12 @@ func TestGetInitializedConsumersReadyToLaunch(t *testing.T) { // no chains to-be-launched exist require.Empty(t, providerKeeper.GetInitializedConsumersReadyToLaunch(ctx, 5)) - providerKeeper.AppendConsumerToBeLaunched(ctx, "consumerId1", time.Unix(10, 0)) - providerKeeper.AppendConsumerToBeLaunched(ctx, "consumerId2", time.Unix(20, 0)) - providerKeeper.AppendConsumerToBeLaunched(ctx, "consumerId3", time.Unix(30, 0)) + err := providerKeeper.AppendConsumerToBeLaunched(ctx, "consumerId1", time.Unix(10, 0)) + require.NoError(t, err) + err = providerKeeper.AppendConsumerToBeLaunched(ctx, "consumerId2", time.Unix(20, 0)) + require.NoError(t, err) + err = providerKeeper.AppendConsumerToBeLaunched(ctx, "consumerId3", time.Unix(30, 0)) + require.NoError(t, err) // time has not yet reached the spawn time of "consumerId1" ctx = ctx.WithBlockTime(time.Unix(9, 999999999)) @@ -507,9 +518,12 @@ func TestGetLaunchedConsumersReadyToStop(t *testing.T) { // no chains to-be-stopped exist require.Empty(t, providerKeeper.GetLaunchedConsumersReadyToStop(ctx, 3)) - providerKeeper.AppendConsumerToBeStopped(ctx, "consumerId1", time.Unix(10, 0)) - providerKeeper.AppendConsumerToBeStopped(ctx, "consumerId2", time.Unix(20, 0)) - providerKeeper.AppendConsumerToBeStopped(ctx, "consumerId3", time.Unix(30, 0)) + err := providerKeeper.AppendConsumerToBeStopped(ctx, "consumerId1", time.Unix(10, 0)) + require.NoError(t, err) + err = providerKeeper.AppendConsumerToBeStopped(ctx, "consumerId2", time.Unix(20, 0)) + require.NoError(t, err) + err = providerKeeper.AppendConsumerToBeStopped(ctx, "consumerId3", time.Unix(30, 0)) + require.NoError(t, err) // time has not yet reached the stop time of "consumerId1" ctx = ctx.WithBlockTime(time.Unix(9, 999999999)) diff --git a/x/ccv/provider/keeper/proposal.go b/x/ccv/provider/keeper/proposal.go index 6d95a1679f..4f0c7d5c0a 100644 --- a/x/ccv/provider/keeper/proposal.go +++ b/x/ccv/provider/keeper/proposal.go @@ -299,7 +299,13 @@ func (k Keeper) BeginBlockInit(ctx sdk.Context) { } // Remove consumer to prevent re-trying launching this chain. // We would only try to re-launch this chain if a new `MsgUpdateConsumer` message is sent. - k.RemoveConsumerToBeLaunched(ctx, consumerId, record.SpawnTime) + err = k.RemoveConsumerToBeLaunched(ctx, consumerId, record.SpawnTime) + if err != nil { + ctx.Logger().Error("could not remove consumer from to-be-launched queue", + "consumerId", consumerId, + "error", err) + continue + } cachedCtx, writeFn := ctx.CacheContext() err = k.LaunchConsumer(cachedCtx, consumerId) @@ -334,14 +340,21 @@ func (k Keeper) BeginBlockCCR(ctx sdk.Context) { err = k.StopConsumerChain(cachedCtx, consumerId, true) if err != nil { - k.Logger(ctx).Info("consumer chain could not be stopped", + k.Logger(ctx).Error("consumer chain could not be stopped", "consumerId", consumerId, "err", err.Error()) continue } k.SetConsumerPhase(cachedCtx, consumerId, types.ConsumerPhase_CONSUMER_PHASE_STOPPED) - k.RemoveConsumerToBeStopped(ctx, consumerId, stopTime) + + err = k.RemoveConsumerToBeStopped(ctx, consumerId, stopTime) + if err != nil { + ctx.Logger().Error("could not remove consumer from to-be-stopped queue", + "consumerId", consumerId, + "error", err) + continue + } // The cached context is created with a new EventManager so we merge the event into the original context ctx.EventManager().EmitEvents(cachedCtx.EventManager().Events()) diff --git a/x/ccv/provider/keeper/proposal_test.go b/x/ccv/provider/keeper/proposal_test.go index 9e5e094eb2..97010b1e93 100644 --- a/x/ccv/provider/keeper/proposal_test.go +++ b/x/ccv/provider/keeper/proposal_test.go @@ -610,13 +610,16 @@ func TestBeginBlockInit(t *testing.T) { providerKeeper.SetConsumerMetadata(ctx, fmt.Sprintf("%d", i), r) } for i, r := range initializationParameters { - providerKeeper.SetConsumerInitializationParameters(ctx, fmt.Sprintf("%d", i), r) + err := providerKeeper.SetConsumerInitializationParameters(ctx, fmt.Sprintf("%d", i), r) + require.NoError(t, err) // set up the chains in their initialized phase, hence they could launch providerKeeper.SetConsumerPhase(ctx, fmt.Sprintf("%d", i), providertypes.ConsumerPhase_CONSUMER_PHASE_INITIALIZED) - providerKeeper.AppendConsumerToBeLaunched(ctx, fmt.Sprintf("%d", i), r.SpawnTime) + err = providerKeeper.AppendConsumerToBeLaunched(ctx, fmt.Sprintf("%d", i), r.SpawnTime) + require.NoError(t, err) } for i, r := range powerShapingParameters { - providerKeeper.SetConsumerPowerShapingParameters(ctx, fmt.Sprintf("%d", i), r) + err := providerKeeper.SetConsumerPowerShapingParameters(ctx, fmt.Sprintf("%d", i), r) + require.NoError(t, err) } // opt in a sample validator so the chain's proposal can successfully execute @@ -677,12 +680,18 @@ func TestBeginBlockCCR(t *testing.T) { chainIds := []string{"chain1", "chain2", "chain3"} consumerIds := []string{"consumerId1", "consumerId2", "consumerId3"} - providerKeeper.SetConsumerStopTime(ctx, consumerIds[0], now.Add(-time.Hour)) - providerKeeper.AppendConsumerToBeStopped(ctx, consumerIds[0], now.Add(-time.Hour)) - providerKeeper.SetConsumerStopTime(ctx, consumerIds[1], now) - providerKeeper.AppendConsumerToBeStopped(ctx, consumerIds[1], now) - providerKeeper.SetConsumerStopTime(ctx, consumerIds[2], now.Add(time.Hour)) - providerKeeper.AppendConsumerToBeStopped(ctx, consumerIds[2], now.Add(time.Hour)) + err := providerKeeper.SetConsumerStopTime(ctx, consumerIds[0], now.Add(-time.Hour)) + require.NoError(t, err) + err = providerKeeper.AppendConsumerToBeStopped(ctx, consumerIds[0], now.Add(-time.Hour)) + require.NoError(t, err) + err = providerKeeper.SetConsumerStopTime(ctx, consumerIds[1], now) + require.NoError(t, err) + err = providerKeeper.AppendConsumerToBeStopped(ctx, consumerIds[1], now) + require.NoError(t, err) + err = providerKeeper.SetConsumerStopTime(ctx, consumerIds[2], now.Add(time.Hour)) + require.NoError(t, err) + err = providerKeeper.AppendConsumerToBeStopped(ctx, consumerIds[2], now.Add(time.Hour)) + require.NoError(t, err) // // Mock expectations @@ -712,13 +721,16 @@ func TestBeginBlockCCR(t *testing.T) { registrationRecord := testkeeper.GetTestConsumerMetadata() providerKeeper.SetConsumerChainId(ctx, consumerId, chainIds[i]) - providerKeeper.SetConsumerMetadata(ctx, consumerId, registrationRecord) - providerKeeper.SetConsumerInitializationParameters(ctx, consumerId, initializationRecord) - providerKeeper.SetConsumerPowerShapingParameters(ctx, consumerId, testkeeper.GetTestPowerShapingParameters()) + err = providerKeeper.SetConsumerMetadata(ctx, consumerId, registrationRecord) + require.NoError(t, err) + err = providerKeeper.SetConsumerInitializationParameters(ctx, consumerId, initializationRecord) + require.NoError(t, err) + err = providerKeeper.SetConsumerPowerShapingParameters(ctx, consumerId, testkeeper.GetTestPowerShapingParameters()) + require.NoError(t, err) providerKeeper.SetConsumerPhase(ctx, consumerId, providertypes.ConsumerPhase_CONSUMER_PHASE_INITIALIZED) providerKeeper.SetClientIdToConsumerId(ctx, "clientID", consumerId) - err := providerKeeper.CreateConsumerClient(ctx, consumerId) + err = providerKeeper.CreateConsumerClient(ctx, consumerId) require.NoError(t, err) err = providerKeeper.SetConsumerChain(ctx, "channelID") require.NoError(t, err) From 520621c192a1bdeb999acd709a2071e6c5190258 Mon Sep 17 00:00:00 2001 From: mpoke Date: Thu, 29 Aug 2024 17:50:08 +0200 Subject: [PATCH 02/13] set the reverse index in SetConsumerClientId --- testutil/keeper/unit_test_helpers.go | 2 +- x/ccv/provider/ibc_module_test.go | 3 +- x/ccv/provider/keeper/grpc_query_test.go | 2 +- x/ccv/provider/keeper/keeper.go | 29 ++++++++++++- x/ccv/provider/keeper/keeper_test.go | 45 ++++++++++++++++++++ x/ccv/provider/keeper/permissionless.go | 22 ---------- x/ccv/provider/keeper/permissionless_test.go | 25 ----------- x/ccv/provider/keeper/proposal.go | 1 - x/ccv/provider/keeper/proposal_test.go | 2 +- x/ccv/provider/keeper/relay_test.go | 3 +- x/ccv/provider/migrations/v8/migrations.go | 11 +++-- 11 files changed, 85 insertions(+), 60 deletions(-) diff --git a/testutil/keeper/unit_test_helpers.go b/testutil/keeper/unit_test_helpers.go index 95c5de2380..181b69ffcc 100644 --- a/testutil/keeper/unit_test_helpers.go +++ b/testutil/keeper/unit_test_helpers.go @@ -239,7 +239,7 @@ func SetupForStoppingConsumerChain(t *testing.T, ctx sdk.Context, err := providerKeeper.CreateConsumerClient(ctx, consumerId) require.NoError(t, err) - providerKeeper.SetClientIdToConsumerId(ctx, "clientID", consumerId) + providerKeeper.SetConsumerClientId(ctx, consumerId, "clientID") err = providerKeeper.SetConsumerChain(ctx, "channelID") require.NoError(t, err) } diff --git a/x/ccv/provider/ibc_module_test.go b/x/ccv/provider/ibc_module_test.go index 2d778daf00..0a06bf6091 100644 --- a/x/ccv/provider/ibc_module_test.go +++ b/x/ccv/provider/ibc_module_test.go @@ -123,7 +123,6 @@ func TestOnChanOpenTry(t *testing.T) { providerKeeper.SetPort(ctx, ccv.ProviderPortID) providerKeeper.SetConsumerClientId(ctx, "consumerId", "clientIdToConsumer") - providerKeeper.SetClientIdToConsumerId(ctx, "clientIdToConsumer", "consumerId") // Instantiate valid params as default. Individual test cases mutate these as needed. params := params{ @@ -313,7 +312,7 @@ func TestOnChanOpenConfirm(t *testing.T) { providerKeeper.SetConsumerIdToChannelId(ctx, "consumerChainID", "existingChannelID") } - providerKeeper.SetClientIdToConsumerId(ctx, "clientID", "consumerChainID") + providerKeeper.SetConsumerClientId(ctx, "consumerChainID", "clientID") providerModule := provider.NewAppModule(&providerKeeper, *keeperParams.ParamsSubspace, keeperParams.StoreKey) diff --git a/x/ccv/provider/keeper/grpc_query_test.go b/x/ccv/provider/keeper/grpc_query_test.go index 9063e1cdd0..c540c4980c 100644 --- a/x/ccv/provider/keeper/grpc_query_test.go +++ b/x/ccv/provider/keeper/grpc_query_test.go @@ -578,7 +578,7 @@ func TestQueryConsumerIdFromClientId(t *testing.T) { require.ErrorContains(t, err, "no known consumer chain") expectedConsumerId := "consumerId" - providerKeeper.SetClientIdToConsumerId(ctx, "clientId", expectedConsumerId) + providerKeeper.SetConsumerClientId(ctx, expectedConsumerId, "clientId") res, err := providerKeeper.QueryConsumerIdFromClientId(ctx, &types.QueryConsumerIdFromClientIdRequest{ClientId: "clientId"}) require.NoError(t, err) diff --git a/x/ccv/provider/keeper/keeper.go b/x/ccv/provider/keeper/keeper.go index 4598ff30c1..7628fe334d 100644 --- a/x/ccv/provider/keeper/keeper.go +++ b/x/ccv/provider/keeper/keeper.go @@ -637,9 +637,18 @@ func (k Keeper) DeletePendingVSCPackets(ctx sdk.Context, consumerId string) { } // SetConsumerClientId sets the client id for the given consumer id -func (k Keeper) SetConsumerClientId(ctx sdk.Context, consumerId, clientID string) { +func (k Keeper) SetConsumerClientId(ctx sdk.Context, consumerId, clientId string) { store := ctx.KVStore(k.storeKey) - store.Set(types.ConsumerIdToClientIdKey(consumerId), []byte(clientID)) + + if prevClientId, found := k.GetConsumerClientId(ctx, consumerId); found { + // delete reverse index + store.Delete(types.ClientIdToConsumerIdKey(prevClientId)) + } + + store.Set(types.ConsumerIdToClientIdKey(consumerId), []byte(clientId)) + + // set the reverse index + store.Set(types.ClientIdToConsumerIdKey(clientId), []byte(consumerId)) } // GetConsumerClientId returns the client id for the given consumer id. @@ -652,9 +661,25 @@ func (k Keeper) GetConsumerClientId(ctx sdk.Context, consumerId string) (string, return string(clientIdBytes), true } +// GetClientIdToConsumerId returns the consumer id associated with this client id +func (k Keeper) GetClientIdToConsumerId(ctx sdk.Context, clientId string) (string, bool) { + store := ctx.KVStore(k.storeKey) + consumerIdBytes := store.Get(types.ClientIdToConsumerIdKey(clientId)) + if consumerIdBytes == nil { + return "", false + } + return string(consumerIdBytes), true +} + // DeleteConsumerClientId removes from the store the client id for the given consumer id. func (k Keeper) DeleteConsumerClientId(ctx sdk.Context, consumerId string) { store := ctx.KVStore(k.storeKey) + + if clientId, found := k.GetConsumerClientId(ctx, consumerId); found { + // delete reverse index + store.Delete(types.ClientIdToConsumerIdKey(clientId)) + } + store.Delete(types.ConsumerIdToClientIdKey(consumerId)) } diff --git a/x/ccv/provider/keeper/keeper_test.go b/x/ccv/provider/keeper/keeper_test.go index 1794c2fba5..6fb169f92c 100644 --- a/x/ccv/provider/keeper/keeper_test.go +++ b/x/ccv/provider/keeper/keeper_test.go @@ -490,3 +490,48 @@ func TestKeeperConsumerParams(t *testing.T) { }) } } + +// TestConsumerClientId tests the getter, setter, and deletion of the client id <> consumer id mappings +func TestConsumerClientId(t *testing.T) { + providerKeeper, ctx, ctrl, _ := testkeeper.GetProviderKeeperAndCtx(t, testkeeper.NewInMemKeeperParams(t)) + defer ctrl.Finish() + + consumerId := "123" + clientIds := []string{"clientId1", "clientId2"} + + _, found := providerKeeper.GetConsumerClientId(ctx, consumerId) + require.False(t, found) + _, found = providerKeeper.GetClientIdToConsumerId(ctx, clientIds[0]) + require.False(t, found) + _, found = providerKeeper.GetClientIdToConsumerId(ctx, clientIds[1]) + require.False(t, found) + + providerKeeper.SetConsumerClientId(ctx, consumerId, clientIds[0]) + res, found := providerKeeper.GetConsumerClientId(ctx, consumerId) + require.True(t, found) + require.Equal(t, clientIds[0], res) + res, found = providerKeeper.GetClientIdToConsumerId(ctx, clientIds[0]) + require.True(t, found) + require.Equal(t, consumerId, res) + _, found = providerKeeper.GetClientIdToConsumerId(ctx, clientIds[1]) + require.False(t, found) + + // overwrite the client ID + providerKeeper.SetConsumerClientId(ctx, consumerId, clientIds[1]) + res, found = providerKeeper.GetConsumerClientId(ctx, consumerId) + require.True(t, found) + require.Equal(t, clientIds[1], res) + res, found = providerKeeper.GetClientIdToConsumerId(ctx, clientIds[1]) + require.True(t, found) + require.Equal(t, consumerId, res) + _, found = providerKeeper.GetClientIdToConsumerId(ctx, clientIds[0]) + require.False(t, found) + + providerKeeper.DeleteConsumerClientId(ctx, consumerId) + _, found = providerKeeper.GetConsumerClientId(ctx, consumerId) + require.False(t, found) + _, found = providerKeeper.GetClientIdToConsumerId(ctx, clientIds[0]) + require.False(t, found) + _, found = providerKeeper.GetClientIdToConsumerId(ctx, clientIds[1]) + require.False(t, found) +} diff --git a/x/ccv/provider/keeper/permissionless.go b/x/ccv/provider/keeper/permissionless.go index f684f8b80d..44e7172994 100644 --- a/x/ccv/provider/keeper/permissionless.go +++ b/x/ccv/provider/keeper/permissionless.go @@ -436,28 +436,6 @@ func (k Keeper) RemoveOptedInConsumerId(ctx sdk.Context, providerAddr types.Prov return nil } -// GetClientIdToConsumerId returns the consumer id associated with this client id -func (k Keeper) GetClientIdToConsumerId(ctx sdk.Context, clientId string) (string, bool) { - store := ctx.KVStore(k.storeKey) - buf := store.Get(types.ClientIdToConsumerIdKey(clientId)) - if buf == nil { - return "", false - } - return string(buf), true -} - -// SetClientIdToConsumerId sets the client id associated with this consumer id -func (k Keeper) SetClientIdToConsumerId(ctx sdk.Context, clientId string, consumerId string) { - store := ctx.KVStore(k.storeKey) - store.Set(types.ClientIdToConsumerIdKey(clientId), []byte(consumerId)) -} - -// DeleteClientIdToConsumerId deletes the client id to consumer id association -func (k Keeper) DeleteClientIdToConsumerId(ctx sdk.Context, clientId string) { - store := ctx.KVStore(k.storeKey) - store.Delete(types.ClientIdToConsumerIdKey(clientId)) -} - // GetInitializedConsumersReadyToLaunch returns the consumer ids of the pending initialized consumer chains // that are ready to launch, i.e., consumer clients to be created. func (k Keeper) GetInitializedConsumersReadyToLaunch(ctx sdk.Context, limit uint32) []string { diff --git a/x/ccv/provider/keeper/permissionless_test.go b/x/ccv/provider/keeper/permissionless_test.go index a797d255ba..94f54acf7c 100644 --- a/x/ccv/provider/keeper/permissionless_test.go +++ b/x/ccv/provider/keeper/permissionless_test.go @@ -449,31 +449,6 @@ func TestGetOptedInConsumerIds(t *testing.T) { require.Empty(t, consumers) } -// TestConsumerChainId tests the getter, setter, and deletion of the client id to consumer id methods -func TestClientIdToConsumerId(t *testing.T) { - providerKeeper, ctx, ctrl, _ := testkeeper.GetProviderKeeperAndCtx(t, testkeeper.NewInMemKeeperParams(t)) - defer ctrl.Finish() - - _, found := providerKeeper.GetClientIdToConsumerId(ctx, "clientId") - require.False(t, found) - - providerKeeper.SetClientIdToConsumerId(ctx, "clientId", "consumerId") - consumerId, found := providerKeeper.GetClientIdToConsumerId(ctx, "clientId") - require.True(t, found) - require.Equal(t, "consumerId", consumerId) - - // assert that overwriting the current consumer id record works - providerKeeper.SetClientIdToConsumerId(ctx, "clientId", "consumerId2") - consumerId, found = providerKeeper.GetClientIdToConsumerId(ctx, "clientId") - require.True(t, found) - require.Equal(t, "consumerId2", consumerId) - - providerKeeper.DeleteClientIdToConsumerId(ctx, "clientId") - consumerId, found = providerKeeper.GetClientIdToConsumerId(ctx, "clientId") - require.False(t, found) - require.Equal(t, "", consumerId) -} - // TestGetInitializedConsumersReadyToLaunch tests that the ready to-be-launched consumer chains are returned func TestGetInitializedConsumersReadyToLaunch(t *testing.T) { providerKeeper, ctx, ctrl, _ := testkeeper.GetProviderKeeperAndCtx(t, testkeeper.NewInMemKeeperParams(t)) diff --git a/x/ccv/provider/keeper/proposal.go b/x/ccv/provider/keeper/proposal.go index 4f0c7d5c0a..ed2df823fd 100644 --- a/x/ccv/provider/keeper/proposal.go +++ b/x/ccv/provider/keeper/proposal.go @@ -90,7 +90,6 @@ func (k Keeper) CreateConsumerClient(ctx sdk.Context, consumerId string) error { return err } k.SetConsumerClientId(ctx, consumerId, clientID) - k.SetClientIdToConsumerId(ctx, clientID, consumerId) k.Logger(ctx).Info("consumer chain launched (client created)", "consumer id", consumerId, diff --git a/x/ccv/provider/keeper/proposal_test.go b/x/ccv/provider/keeper/proposal_test.go index 97010b1e93..f65fae963a 100644 --- a/x/ccv/provider/keeper/proposal_test.go +++ b/x/ccv/provider/keeper/proposal_test.go @@ -728,7 +728,7 @@ func TestBeginBlockCCR(t *testing.T) { err = providerKeeper.SetConsumerPowerShapingParameters(ctx, consumerId, testkeeper.GetTestPowerShapingParameters()) require.NoError(t, err) providerKeeper.SetConsumerPhase(ctx, consumerId, providertypes.ConsumerPhase_CONSUMER_PHASE_INITIALIZED) - providerKeeper.SetClientIdToConsumerId(ctx, "clientID", consumerId) + providerKeeper.SetConsumerClientId(ctx, consumerId, "clientID") err = providerKeeper.CreateConsumerClient(ctx, consumerId) require.NoError(t, err) diff --git a/x/ccv/provider/keeper/relay_test.go b/x/ccv/provider/keeper/relay_test.go index fb27247263..b539c9a008 100644 --- a/x/ccv/provider/keeper/relay_test.go +++ b/x/ccv/provider/keeper/relay_test.go @@ -501,10 +501,9 @@ func TestSendVSCPacketsToChainFailure(t *testing.T) { gomock.InOrder(mockCalls...) // Execute setup - providerKeeper.SetClientIdToConsumerId(ctx, "clientID", "consumerChainID") + providerKeeper.SetConsumerClientId(ctx, "consumerChainID", "clientID") err := providerKeeper.SetConsumerChain(ctx, "channelID") require.NoError(t, err) - providerKeeper.SetConsumerClientId(ctx, "consumerChainID", "clientID") // No panic should occur, StopConsumerChain should be called providerKeeper.SendVSCPacketsToChain(ctx, "consumerChainID", "CCVChannelID") diff --git a/x/ccv/provider/migrations/v8/migrations.go b/x/ccv/provider/migrations/v8/migrations.go index 916e3d0a22..317121f777 100644 --- a/x/ccv/provider/migrations/v8/migrations.go +++ b/x/ccv/provider/migrations/v8/migrations.go @@ -266,15 +266,20 @@ func MigrateLaunchedConsumerChains(ctx sdk.Context, store storetypes.KVStore, pk // This is to migrate everything under `ProviderConsAddrToOptedInConsumerIdsKey` // `OptedIn` was already re-keyed earlier (see above) and hence we can use `consumerId` here. for _, providerConsAddr := range pk.GetAllOptedIn(ctx, consumerId) { - pk.AppendOptedInConsumerId(ctx, providerConsAddr, consumerId) + err := pk.AppendOptedInConsumerId(ctx, providerConsAddr, consumerId) + if err != nil { + return err + } } // set clientId -> consumerId mapping - clientId, found := pk.GetConsumerClientId(ctx, consumerId) // consumer to client was already re-keyed so we can use `consumerId` here + // consumer to client was already re-keyed so we can use `consumerId` here + // however, during the rekeying, the reverse index was not set + clientId, found := pk.GetConsumerClientId(ctx, consumerId) if !found { return errorsmod.Wrapf(ccv.ErrInvalidConsumerState, "cannot find client ID associated with consumer ID: %s", consumerId) } - pk.SetClientIdToConsumerId(ctx, clientId, consumerId) + pk.SetConsumerClientId(ctx, consumerId, clientId) } return nil From afb780d9e8a434b69f365f4fbf489179f98d99cf Mon Sep 17 00:00:00 2001 From: mpoke Date: Thu, 29 Aug 2024 19:10:47 +0200 Subject: [PATCH 03/13] rename method --- x/ccv/provider/keeper/permissionless.go | 13 ++++---- x/ccv/provider/keeper/permissionless_test.go | 32 ++++++++++---------- x/ccv/provider/keeper/proposal.go | 4 +-- 3 files changed, 25 insertions(+), 24 deletions(-) diff --git a/x/ccv/provider/keeper/permissionless.go b/x/ccv/provider/keeper/permissionless.go index 44e7172994..c530023950 100644 --- a/x/ccv/provider/keeper/permissionless.go +++ b/x/ccv/provider/keeper/permissionless.go @@ -371,6 +371,7 @@ func (k Keeper) GetOptedInConsumerIds(ctx sdk.Context, providerAddr types.Provid } // AppendOptedInConsumerId appends given consumer id to the list of consumers that validator has opted in +// TODO (PERMISSIONLESS) -- combine it with SetOptedIn func (k Keeper) AppendOptedInConsumerId(ctx sdk.Context, providerAddr types.ProviderConsAddress, consumerId string) error { store := ctx.KVStore(k.storeKey) @@ -436,9 +437,9 @@ func (k Keeper) RemoveOptedInConsumerId(ctx sdk.Context, providerAddr types.Prov return nil } -// GetInitializedConsumersReadyToLaunch returns the consumer ids of the pending initialized consumer chains +// GetConsumersReadyToLaunch returns the consumer ids of the pending initialized consumer chains // that are ready to launch, i.e., consumer clients to be created. -func (k Keeper) GetInitializedConsumersReadyToLaunch(ctx sdk.Context, limit uint32) []string { +func (k Keeper) GetConsumersReadyToLaunch(ctx sdk.Context, limit uint32) []string { store := ctx.KVStore(k.storeKey) spawnTimeToConsumerIdsKeyPrefix := types.SpawnTimeToConsumerIdsKeyPrefix() @@ -447,7 +448,7 @@ func (k Keeper) GetInitializedConsumersReadyToLaunch(ctx sdk.Context, limit uint result := []string{} for ; iterator.Valid(); iterator.Next() { - spawnTime, err := types.ParseTime(types.SpawnTimeToConsumerIdsKeyPrefix(), iterator.Key()) + spawnTime, err := types.ParseTime(spawnTimeToConsumerIdsKeyPrefix, iterator.Key()) if err != nil { k.Logger(ctx).Error("failed to parse spawn time", "error", err) @@ -479,9 +480,9 @@ func (k Keeper) GetInitializedConsumersReadyToLaunch(ctx sdk.Context, limit uint return result } -// GetLaunchedConsumersReadyToStop returns the consumer ids of the pending launched consumer chains +// GetConsumersReadyToStop returns the consumer ids of the pending launched consumer chains // that are ready to stop -func (k Keeper) GetLaunchedConsumersReadyToStop(ctx sdk.Context, limit uint32) []string { +func (k Keeper) GetConsumersReadyToStop(ctx sdk.Context, limit uint32) []string { store := ctx.KVStore(k.storeKey) stopTimeToConsumerIdsKeyPrefix := types.StopTimeToConsumerIdsKeyPrefix() @@ -490,7 +491,7 @@ func (k Keeper) GetLaunchedConsumersReadyToStop(ctx sdk.Context, limit uint32) [ result := []string{} for ; iterator.Valid(); iterator.Next() { - stopTime, err := types.ParseTime(types.StopTimeToConsumerIdsKeyPrefix(), iterator.Key()) + stopTime, err := types.ParseTime(stopTimeToConsumerIdsKeyPrefix, iterator.Key()) if err != nil { k.Logger(ctx).Error("failed to parse stop time", "error", err) diff --git a/x/ccv/provider/keeper/permissionless_test.go b/x/ccv/provider/keeper/permissionless_test.go index 94f54acf7c..71fcc8c51d 100644 --- a/x/ccv/provider/keeper/permissionless_test.go +++ b/x/ccv/provider/keeper/permissionless_test.go @@ -449,13 +449,13 @@ func TestGetOptedInConsumerIds(t *testing.T) { require.Empty(t, consumers) } -// TestGetInitializedConsumersReadyToLaunch tests that the ready to-be-launched consumer chains are returned -func TestGetInitializedConsumersReadyToLaunch(t *testing.T) { +// TestGetConsumersReadyToLaunch tests that the ready to-be-launched consumer chains are returned +func TestGetConsumersReadyToLaunch(t *testing.T) { providerKeeper, ctx, ctrl, _ := testkeeper.GetProviderKeeperAndCtx(t, testkeeper.NewInMemKeeperParams(t)) defer ctrl.Finish() // no chains to-be-launched exist - require.Empty(t, providerKeeper.GetInitializedConsumersReadyToLaunch(ctx, 5)) + require.Empty(t, providerKeeper.GetConsumersReadyToLaunch(ctx, 5)) err := providerKeeper.AppendConsumerToBeLaunched(ctx, "consumerId1", time.Unix(10, 0)) require.NoError(t, err) @@ -466,32 +466,32 @@ func TestGetInitializedConsumersReadyToLaunch(t *testing.T) { // time has not yet reached the spawn time of "consumerId1" ctx = ctx.WithBlockTime(time.Unix(9, 999999999)) - require.Empty(t, providerKeeper.GetInitializedConsumersReadyToLaunch(ctx, 3)) + require.Empty(t, providerKeeper.GetConsumersReadyToLaunch(ctx, 3)) // time has reached the spawn time of "consumerId1" ctx = ctx.WithBlockTime(time.Unix(10, 0)) - require.Equal(t, []string{"consumerId1"}, providerKeeper.GetInitializedConsumersReadyToLaunch(ctx, 3)) + require.Equal(t, []string{"consumerId1"}, providerKeeper.GetConsumersReadyToLaunch(ctx, 3)) // time has reached the spawn time of "consumerId1" and "consumerId2" ctx = ctx.WithBlockTime(time.Unix(20, 0)) - require.Equal(t, []string{"consumerId1", "consumerId2"}, providerKeeper.GetInitializedConsumersReadyToLaunch(ctx, 3)) + require.Equal(t, []string{"consumerId1", "consumerId2"}, providerKeeper.GetConsumersReadyToLaunch(ctx, 3)) // time has reached the spawn time of all chains ctx = ctx.WithBlockTime(time.Unix(30, 0)) - require.Equal(t, []string{"consumerId1", "consumerId2", "consumerId3"}, providerKeeper.GetInitializedConsumersReadyToLaunch(ctx, 3)) + require.Equal(t, []string{"consumerId1", "consumerId2", "consumerId3"}, providerKeeper.GetConsumersReadyToLaunch(ctx, 3)) // limit the number of returned consumer chains - require.Equal(t, []string{}, providerKeeper.GetInitializedConsumersReadyToLaunch(ctx, 0)) - require.Equal(t, []string{"consumerId1"}, providerKeeper.GetInitializedConsumersReadyToLaunch(ctx, 1)) - require.Equal(t, []string{"consumerId1", "consumerId2"}, providerKeeper.GetInitializedConsumersReadyToLaunch(ctx, 2)) + require.Equal(t, []string{}, providerKeeper.GetConsumersReadyToLaunch(ctx, 0)) + require.Equal(t, []string{"consumerId1"}, providerKeeper.GetConsumersReadyToLaunch(ctx, 1)) + require.Equal(t, []string{"consumerId1", "consumerId2"}, providerKeeper.GetConsumersReadyToLaunch(ctx, 2)) } -func TestGetLaunchedConsumersReadyToStop(t *testing.T) { +func TestGetConsumersReadyToStop(t *testing.T) { providerKeeper, ctx, ctrl, _ := testkeeper.GetProviderKeeperAndCtx(t, testkeeper.NewInMemKeeperParams(t)) defer ctrl.Finish() // no chains to-be-stopped exist - require.Empty(t, providerKeeper.GetLaunchedConsumersReadyToStop(ctx, 3)) + require.Empty(t, providerKeeper.GetConsumersReadyToStop(ctx, 3)) err := providerKeeper.AppendConsumerToBeStopped(ctx, "consumerId1", time.Unix(10, 0)) require.NoError(t, err) @@ -502,19 +502,19 @@ func TestGetLaunchedConsumersReadyToStop(t *testing.T) { // time has not yet reached the stop time of "consumerId1" ctx = ctx.WithBlockTime(time.Unix(9, 999999999)) - require.Empty(t, providerKeeper.GetLaunchedConsumersReadyToStop(ctx, 3)) + require.Empty(t, providerKeeper.GetConsumersReadyToStop(ctx, 3)) // time has reached the stop time of "consumerId1" ctx = ctx.WithBlockTime(time.Unix(10, 0)) - require.Equal(t, []string{"consumerId1"}, providerKeeper.GetLaunchedConsumersReadyToStop(ctx, 3)) + require.Equal(t, []string{"consumerId1"}, providerKeeper.GetConsumersReadyToStop(ctx, 3)) // time has reached the stop time of "consumerId1" and "consumerId2" ctx = ctx.WithBlockTime(time.Unix(20, 0)) - require.Equal(t, []string{"consumerId1", "consumerId2"}, providerKeeper.GetLaunchedConsumersReadyToStop(ctx, 3)) + require.Equal(t, []string{"consumerId1", "consumerId2"}, providerKeeper.GetConsumersReadyToStop(ctx, 3)) // time has reached the stop time of all chains ctx = ctx.WithBlockTime(time.Unix(30, 0)) - require.Equal(t, []string{"consumerId1", "consumerId2", "consumerId3"}, providerKeeper.GetLaunchedConsumersReadyToStop(ctx, 3)) + require.Equal(t, []string{"consumerId1", "consumerId2", "consumerId3"}, providerKeeper.GetConsumersReadyToStop(ctx, 3)) } func TestUpdateAllowlist(t *testing.T) { diff --git a/x/ccv/provider/keeper/proposal.go b/x/ccv/provider/keeper/proposal.go index ed2df823fd..6d773a22e0 100644 --- a/x/ccv/provider/keeper/proposal.go +++ b/x/ccv/provider/keeper/proposal.go @@ -288,7 +288,7 @@ func (k Keeper) MakeConsumerGenesis( // in which the spawn time has passed func (k Keeper) BeginBlockInit(ctx sdk.Context) { // TODO (PERMISSIONLESS): we can parameterize the limit - for _, consumerId := range k.GetInitializedConsumersReadyToLaunch(ctx, 200) { + for _, consumerId := range k.GetConsumersReadyToLaunch(ctx, 200) { record, err := k.GetConsumerInitializationParameters(ctx, consumerId) if err != nil { ctx.Logger().Error("could not retrieve initialization record", @@ -325,7 +325,7 @@ func (k Keeper) BeginBlockInit(ctx sdk.Context) { // BeginBlockCCR iterates over the pending consumer proposals and stop/removes the chain if the stop time has passed func (k Keeper) BeginBlockCCR(ctx sdk.Context) { // TODO (PERMISSIONLESS): parameterize the limit - for _, consumerId := range k.GetLaunchedConsumersReadyToStop(ctx, 200) { + for _, consumerId := range k.GetConsumersReadyToStop(ctx, 200) { // stop consumer chain in a cached context to handle errors cachedCtx, writeFn := ctx.CacheContext() From 08d861f8c39a1abfeb05076978d2e6ca24a85fa4 Mon Sep 17 00:00:00 2001 From: mpoke Date: Thu, 29 Aug 2024 19:48:21 +0200 Subject: [PATCH 04/13] create consumer_lifecycle.go with launch/stop logic --- x/ccv/provider/keeper/consumer_lifecycle.go | 609 ++++++++++++ .../keeper/consumer_lifecycle_test.go | 900 ++++++++++++++++++ x/ccv/provider/keeper/permissionless.go | 250 ----- x/ccv/provider/keeper/permissionless_test.go | 219 ----- x/ccv/provider/keeper/proposal.go | 350 ------- x/ccv/provider/keeper/proposal_test.go | 676 ------------- x/ccv/provider/module.go | 4 +- 7 files changed, 1511 insertions(+), 1497 deletions(-) create mode 100644 x/ccv/provider/keeper/consumer_lifecycle.go create mode 100644 x/ccv/provider/keeper/consumer_lifecycle_test.go diff --git a/x/ccv/provider/keeper/consumer_lifecycle.go b/x/ccv/provider/keeper/consumer_lifecycle.go new file mode 100644 index 0000000000..65cdca8b6a --- /dev/null +++ b/x/ccv/provider/keeper/consumer_lifecycle.go @@ -0,0 +1,609 @@ +package keeper + +import ( + "fmt" + "time" + + tmtypes "github.com/cometbft/cometbft/types" + + errorsmod "cosmossdk.io/errors" + storetypes "cosmossdk.io/store/types" + + sdk "github.com/cosmos/cosmos-sdk/types" + sdkerrors "github.com/cosmos/cosmos-sdk/types/errors" + stakingtypes "github.com/cosmos/cosmos-sdk/x/staking/types" + + clienttypes "github.com/cosmos/ibc-go/v8/modules/core/02-client/types" + channeltypes "github.com/cosmos/ibc-go/v8/modules/core/04-channel/types" + commitmenttypes "github.com/cosmos/ibc-go/v8/modules/core/23-commitment/types" + ibctmtypes "github.com/cosmos/ibc-go/v8/modules/light-clients/07-tendermint" + + "github.com/cosmos/interchain-security/v5/x/ccv/provider/types" + ccv "github.com/cosmos/interchain-security/v5/x/ccv/types" +) + +// BeginBlockLaunchConsumers launches initialized consumers that are ready to launch +func (k Keeper) BeginBlockLaunchConsumers(ctx sdk.Context) { + // TODO (PERMISSIONLESS): we can parameterize the limit + for _, consumerId := range k.GetConsumersReadyToLaunch(ctx, 200) { + record, err := k.GetConsumerInitializationParameters(ctx, consumerId) + if err != nil { + ctx.Logger().Error("could not retrieve initialization record", + "consumerId", consumerId, + "error", err) + continue + } + // Remove consumer to prevent re-trying launching this chain. + // We would only try to re-launch this chain if a new `MsgUpdateConsumer` message is sent. + err = k.RemoveConsumerToBeLaunched(ctx, consumerId, record.SpawnTime) + if err != nil { + ctx.Logger().Error("could not remove consumer from to-be-launched queue", + "consumerId", consumerId, + "error", err) + continue + } + + cachedCtx, writeFn := ctx.CacheContext() + err = k.LaunchConsumer(cachedCtx, consumerId) + if err != nil { + ctx.Logger().Error("could not launch chain", + "consumerId", consumerId, + "error", err) + continue + } + k.SetConsumerPhase(cachedCtx, consumerId, types.ConsumerPhase_CONSUMER_PHASE_LAUNCHED) + + // the cached context is created with a new EventManager, so we merge the events into the original context + ctx.EventManager().EmitEvents(cachedCtx.EventManager().Events()) + writeFn() + } +} + +// GetConsumersReadyToLaunch returns the consumer ids of the pending initialized consumer chains +// that are ready to launch, i.e., consumer clients to be created. +func (k Keeper) GetConsumersReadyToLaunch(ctx sdk.Context, limit uint32) []string { + store := ctx.KVStore(k.storeKey) + + spawnTimeToConsumerIdsKeyPrefix := types.SpawnTimeToConsumerIdsKeyPrefix() + iterator := storetypes.KVStorePrefixIterator(store, []byte{spawnTimeToConsumerIdsKeyPrefix}) + defer iterator.Close() + + result := []string{} + for ; iterator.Valid(); iterator.Next() { + spawnTime, err := types.ParseTime(spawnTimeToConsumerIdsKeyPrefix, iterator.Key()) + if err != nil { + k.Logger(ctx).Error("failed to parse spawn time", + "error", err) + continue + } + if spawnTime.After(ctx.BlockTime()) { + return result + } + + // if current block time is equal to or after spawnTime, and the chain is initialized, we can launch the chain + consumerIds, err := k.GetConsumersToBeLaunched(ctx, spawnTime) + if err != nil { + k.Logger(ctx).Error("failed to retrieve consumers to launch", + "spawn time", spawnTime, + "error", err) + continue + } + if len(result)+len(consumerIds.Ids) >= int(limit) { + remainingConsumerIds := len(result) + len(consumerIds.Ids) - int(limit) + if len(consumerIds.Ids[:len(consumerIds.Ids)-remainingConsumerIds]) == 0 { + return result + } + return append(result, consumerIds.Ids[:len(consumerIds.Ids)-remainingConsumerIds]...) + } else { + result = append(result, consumerIds.Ids...) + } + } + + return result +} + +// LaunchConsumer launches the chain with the provided consumer id by creating the consumer client and the respective +// consumer genesis file +func (k Keeper) LaunchConsumer(ctx sdk.Context, consumerId string) error { + err := k.CreateConsumerClient(ctx, consumerId) + if err != nil { + return err + } + + consumerGenesis, found := k.GetConsumerGenesis(ctx, consumerId) + if !found { + return errorsmod.Wrapf(types.ErrNoConsumerGenesis, "consumer genesis could not be found for consumer id: %s", consumerId) + } + + if len(consumerGenesis.Provider.InitialValSet) == 0 { + return errorsmod.Wrapf(types.ErrInvalidConsumerGenesis, "consumer genesis initial validator set is empty - no validators opted in consumer id: %s", consumerId) + } + + return nil +} + +// CreateConsumerClient will create the CCV client for the given consumer chain. The CCV channel must be built +// on top of the CCV client to ensure connection with the right consumer chain. +func (k Keeper) CreateConsumerClient(ctx sdk.Context, consumerId string) error { + initializationRecord, err := k.GetConsumerInitializationParameters(ctx, consumerId) + if err != nil { + return err + } + + phase := k.GetConsumerPhase(ctx, consumerId) + if phase != types.ConsumerPhase_CONSUMER_PHASE_INITIALIZED { + return errorsmod.Wrapf(types.ErrInvalidPhase, + "cannot create client for consumer chain that is not in the Initialized phase but in phase %d: %s", phase, consumerId) + } + + chainId, err := k.GetConsumerChainId(ctx, consumerId) + if err != nil { + return err + } + + // Set minimum height for equivocation evidence from this consumer chain + k.SetEquivocationEvidenceMinHeight(ctx, consumerId, initializationRecord.InitialHeight.RevisionHeight) + + // Consumers start out with the unbonding period from the consumer addition prop + consumerUnbondingPeriod := initializationRecord.UnbondingPeriod + + // Create client state by getting template client from parameters and filling in zeroed fields from proposal. + clientState := k.GetTemplateClient(ctx) + clientState.ChainId = chainId + clientState.LatestHeight = initializationRecord.InitialHeight + + trustPeriod, err := ccv.CalculateTrustPeriod(consumerUnbondingPeriod, k.GetTrustingPeriodFraction(ctx)) + if err != nil { + return err + } + clientState.TrustingPeriod = trustPeriod + clientState.UnbondingPeriod = consumerUnbondingPeriod + + consumerGen, validatorSetHash, err := k.MakeConsumerGenesis(ctx, consumerId) + if err != nil { + return err + } + err = k.SetConsumerGenesis(ctx, consumerId, consumerGen) + if err != nil { + return err + } + + // Create consensus state + consensusState := ibctmtypes.NewConsensusState( + ctx.BlockTime(), + commitmenttypes.NewMerkleRoot([]byte(ibctmtypes.SentinelRoot)), + validatorSetHash, // use the hash of the updated initial valset + ) + + clientID, err := k.clientKeeper.CreateClient(ctx, clientState, consensusState) + if err != nil { + return err + } + k.SetConsumerClientId(ctx, consumerId, clientID) + + k.Logger(ctx).Info("consumer chain launched (client created)", + "consumer id", consumerId, + "client id", clientID, + ) + + ctx.EventManager().EmitEvent( + sdk.NewEvent( + types.EventTypeConsumerClientCreated, + sdk.NewAttribute(sdk.AttributeKeyModule, types.ModuleName), + sdk.NewAttribute(ccv.AttributeChainID, consumerId), + sdk.NewAttribute(clienttypes.AttributeKeyClientID, clientID), + sdk.NewAttribute(types.AttributeInitialHeight, initializationRecord.InitialHeight.String()), + sdk.NewAttribute(types.AttributeTrustingPeriod, clientState.TrustingPeriod.String()), + sdk.NewAttribute(types.AttributeUnbondingPeriod, clientState.UnbondingPeriod.String()), + ), + ) + + return nil +} + +// MakeConsumerGenesis returns the created consumer genesis state for consumer chain `consumerId`, +// as well as the validator hash of the initial validator set of the consumer chain +func (k Keeper) MakeConsumerGenesis( + ctx sdk.Context, + consumerId string, +) (gen ccv.ConsumerGenesisState, nextValidatorsHash []byte, err error) { + initializationRecord, err := k.GetConsumerInitializationParameters(ctx, consumerId) + if err != nil { + return gen, nil, errorsmod.Wrapf(ccv.ErrInvalidConsumerState, + "cannot retrieve initialization parameters: %s", err.Error()) + } + powerShapingParameters, err := k.GetConsumerPowerShapingParameters(ctx, consumerId) + if err != nil { + return gen, nil, errorsmod.Wrapf(ccv.ErrInvalidConsumerState, + "cannot retrieve power shaping parameters: %s", err.Error()) + } + + providerUnbondingPeriod, err := k.stakingKeeper.UnbondingTime(ctx) + if err != nil { + return gen, nil, errorsmod.Wrapf(types.ErrNoUnbondingTime, "unbonding time not found: %s", err) + } + height := clienttypes.GetSelfHeight(ctx) + + clientState := k.GetTemplateClient(ctx) + // this is the counter party chain ID for the consumer + clientState.ChainId = ctx.ChainID() + // this is the latest height the client was updated at, i.e., + // the height of the latest consensus state (see below) + clientState.LatestHeight = height + trustPeriod, err := ccv.CalculateTrustPeriod(providerUnbondingPeriod, k.GetTrustingPeriodFraction(ctx)) + if err != nil { + return gen, nil, errorsmod.Wrapf(sdkerrors.ErrInvalidHeight, "error %s calculating trusting_period for: %s", err, height) + } + clientState.TrustingPeriod = trustPeriod + clientState.UnbondingPeriod = providerUnbondingPeriod + + consState, err := k.clientKeeper.GetSelfConsensusState(ctx, height) + if err != nil { + return gen, nil, errorsmod.Wrapf(clienttypes.ErrConsensusStateNotFound, "error %s getting self consensus state for: %s", err, height) + } + + // get the bonded validators from the staking module + bondedValidators, err := k.GetLastBondedValidators(ctx) + if err != nil { + return gen, nil, errorsmod.Wrapf(stakingtypes.ErrNoValidatorFound, "error getting last bonded validators: %s", err) + } + + minPower := int64(0) + if powerShapingParameters.Top_N > 0 { + // get the consensus active validators + // we do not want to base the power calculation for the top N + // on inactive validators, too, since the top N will be a percentage of the active set power + // otherwise, it could be that inactive validators are forced to validate + activeValidators, err := k.GetLastProviderConsensusActiveValidators(ctx) + if err != nil { + return gen, nil, errorsmod.Wrapf(stakingtypes.ErrNoValidatorFound, "error getting last active bonded validators: %s", err) + } + + // in a Top-N chain, we automatically opt in all validators that belong to the top N + minPower, err = k.ComputeMinPowerInTopN(ctx, activeValidators, powerShapingParameters.Top_N) + if err != nil { + return gen, nil, err + } + // log the minimum power in top N + k.Logger(ctx).Info("minimum power in top N at consumer genesis", + "consumerId", consumerId, + "minPower", minPower, + ) + k.OptInTopNValidators(ctx, consumerId, activeValidators, minPower) + k.SetMinimumPowerInTopN(ctx, consumerId, minPower) + } + + // need to use the bondedValidators, not activeValidators, here since the chain might be opt-in and allow inactive vals + nextValidators := k.ComputeNextValidators(ctx, consumerId, bondedValidators, powerShapingParameters, minPower) + k.SetConsumerValSet(ctx, consumerId, nextValidators) + + // get the initial updates with the latest set consumer public keys + initialUpdatesWithConsumerKeys := DiffValidators([]types.ConsensusValidator{}, nextValidators) + + // Get a hash of the consumer validator set from the update with applied consumer assigned keys + updatesAsValSet, err := tmtypes.PB2TM.ValidatorUpdates(initialUpdatesWithConsumerKeys) + if err != nil { + return gen, nil, fmt.Errorf("unable to create validator set from updates computed from key assignment in MakeConsumerGenesis: %s", err) + } + hash := tmtypes.NewValidatorSet(updatesAsValSet).Hash() + + consumerGenesisParams := ccv.NewParams( + true, + initializationRecord.BlocksPerDistributionTransmission, + initializationRecord.DistributionTransmissionChannel, + "", // providerFeePoolAddrStr, + initializationRecord.CcvTimeoutPeriod, + initializationRecord.TransferTimeoutPeriod, + initializationRecord.ConsumerRedistributionFraction, + initializationRecord.HistoricalEntries, + initializationRecord.UnbondingPeriod, + []string{}, + []string{}, + ccv.DefaultRetryDelayPeriod, + ) + + gen = *ccv.NewInitialConsumerGenesisState( + clientState, + consState.(*ibctmtypes.ConsensusState), + initialUpdatesWithConsumerKeys, + consumerGenesisParams, + ) + return gen, hash, nil +} + +// BeginBlockStopConsumers iterates over the pending consumer proposals and stop/removes the chain if the stop time has passed +func (k Keeper) BeginBlockStopConsumers(ctx sdk.Context) { + // TODO (PERMISSIONLESS): parameterize the limit + for _, consumerId := range k.GetConsumersReadyToStop(ctx, 200) { + // stop consumer chain in a cached context to handle errors + cachedCtx, writeFn := ctx.CacheContext() + + stopTime, err := k.GetConsumerStopTime(ctx, consumerId) + if err != nil { + k.Logger(ctx).Error("chain could not be stopped", + "consumerId", consumerId, + "err", err.Error()) + continue + } + + err = k.StopConsumerChain(cachedCtx, consumerId, true) + if err != nil { + k.Logger(ctx).Error("consumer chain could not be stopped", + "consumerId", consumerId, + "err", err.Error()) + continue + } + + k.SetConsumerPhase(cachedCtx, consumerId, types.ConsumerPhase_CONSUMER_PHASE_STOPPED) + + err = k.RemoveConsumerToBeStopped(ctx, consumerId, stopTime) + if err != nil { + ctx.Logger().Error("could not remove consumer from to-be-stopped queue", + "consumerId", consumerId, + "error", err) + continue + } + + // The cached context is created with a new EventManager so we merge the event into the original context + ctx.EventManager().EmitEvents(cachedCtx.EventManager().Events()) + + writeFn() + + k.Logger(ctx).Info("executed consumer removal", + "consumer id", consumerId, + "stop time", stopTime, + ) + } +} + +// GetConsumersReadyToStop returns the consumer ids of the pending launched consumer chains +// that are ready to stop +func (k Keeper) GetConsumersReadyToStop(ctx sdk.Context, limit uint32) []string { + store := ctx.KVStore(k.storeKey) + + stopTimeToConsumerIdsKeyPrefix := types.StopTimeToConsumerIdsKeyPrefix() + iterator := storetypes.KVStorePrefixIterator(store, []byte{stopTimeToConsumerIdsKeyPrefix}) + defer iterator.Close() + + result := []string{} + for ; iterator.Valid(); iterator.Next() { + stopTime, err := types.ParseTime(stopTimeToConsumerIdsKeyPrefix, iterator.Key()) + if err != nil { + k.Logger(ctx).Error("failed to parse stop time", + "error", err) + continue + } + if stopTime.After(ctx.BlockTime()) { + return result + } + + consumers, err := k.GetConsumersToBeStopped(ctx, stopTime) + if err != nil { + k.Logger(ctx).Error("failed to retrieve consumers to stop", + "stop time", stopTime, + "error", err) + continue + } + if len(result)+len(consumers.Ids) >= int(limit) { + remainingConsumerIds := len(result) + len(consumers.Ids) - int(limit) + if len(consumers.Ids[:len(consumers.Ids)-remainingConsumerIds]) == 0 { + return result + } + return append(result, consumers.Ids[:len(consumers.Ids)-remainingConsumerIds]...) + } else { + result = append(result, consumers.Ids...) + } + } + + return result +} + +// StopConsumerChain cleans up the states for the given consumer id +func (k Keeper) StopConsumerChain(ctx sdk.Context, consumerId string, closeChan bool) (err error) { + // check that a client for consumerId exists + // TODO (PERMISSIONLESS): change to use phases instead + if _, found := k.GetConsumerClientId(ctx, consumerId); !found { + return errorsmod.Wrap(types.ErrConsumerChainNotFound, + fmt.Sprintf("cannot stop non-existent consumer chain: %s", consumerId)) + } + + // clean up states + k.DeleteConsumerClientId(ctx, consumerId) + k.DeleteConsumerGenesis(ctx, consumerId) + // Note: this call panics if the key assignment state is invalid + k.DeleteKeyAssignments(ctx, consumerId) + k.DeleteMinimumPowerInTopN(ctx, consumerId) + k.DeleteEquivocationEvidenceMinHeight(ctx, consumerId) + + // close channel and delete the mappings between chain ID and channel ID + if channelID, found := k.GetConsumerIdToChannelId(ctx, consumerId); found { + if closeChan { + // Close the channel for the given channel ID on the condition + // that the channel exists and isn't already in the CLOSED state + channel, found := k.channelKeeper.GetChannel(ctx, ccv.ProviderPortID, channelID) + if found && channel.State != channeltypes.CLOSED { + err := k.chanCloseInit(ctx, channelID) + if err != nil { + k.Logger(ctx).Error("channel to consumer chain could not be closed", + "consumerId", consumerId, + "channelID", channelID, + "error", err.Error(), + ) + } + } + } + k.DeleteConsumerIdToChannelId(ctx, consumerId) + k.DeleteChannelIdToConsumerId(ctx, channelID) + } + + // delete consumer commission rate + provAddrs := k.GetAllCommissionRateValidators(ctx, consumerId) + for _, addr := range provAddrs { + k.DeleteConsumerCommissionRate(ctx, consumerId, addr) + } + + k.DeleteInitChainHeight(ctx, consumerId) + k.DeleteSlashAcks(ctx, consumerId) + k.DeletePendingVSCPackets(ctx, consumerId) + + k.DeleteConsumerMetadata(ctx, consumerId) + k.DeleteConsumerPowerShapingParameters(ctx, consumerId) + k.DeleteConsumerPowerShapingParameters(ctx, consumerId) + k.DeleteAllowlist(ctx, consumerId) + k.DeleteDenylist(ctx, consumerId) + k.DeleteAllOptedIn(ctx, consumerId) + k.DeleteConsumerValSet(ctx, consumerId) + + // TODO (PERMISSIONLESS) add newly-added state to be deleted + + k.Logger(ctx).Info("consumer chain removed from provider", "consumerId", consumerId) + + return nil +} + +// +// Setters and Getters +// + +// GetConsumerStopTime returns the stop time associated with the to-be-stopped chain with consumer id +func (k Keeper) GetConsumerStopTime(ctx sdk.Context, consumerId string) (time.Time, error) { + store := ctx.KVStore(k.storeKey) + buf := store.Get(types.ConsumerIdToStopTimeKey(consumerId)) + if buf == nil { + return time.Time{}, fmt.Errorf("failed to retrieve stop time for consumer id (%s)", consumerId) + } + var time time.Time + if err := time.UnmarshalBinary(buf); err != nil { + return time, fmt.Errorf("failed to unmarshal stop time for consumer id (%s): %w", consumerId, err) + } + return time, nil +} + +// SetConsumerStopTime sets the stop time associated with this consumer id +func (k Keeper) SetConsumerStopTime(ctx sdk.Context, consumerId string, stopTime time.Time) error { + store := ctx.KVStore(k.storeKey) + buf, err := stopTime.MarshalBinary() + if err != nil { + return fmt.Errorf("failed to marshal stop time (%+v) for consumer id (%s): %w", stopTime, consumerId, err) + } + store.Set(types.ConsumerIdToStopTimeKey(consumerId), buf) + return nil +} + +// DeleteConsumerStopTime deletes the stop time associated with this consumer id +func (k Keeper) DeleteConsumerStopTime(ctx sdk.Context, consumerId string) { + store := ctx.KVStore(k.storeKey) + store.Delete(types.ConsumerIdToStopTimeKey(consumerId)) +} + +// getConsumerIdsBasedOnTime returns all the consumer ids stored under this specific `key(time)` +func (k Keeper) getConsumerIdsBasedOnTime(ctx sdk.Context, key func(time.Time) []byte, time time.Time) (types.ConsumerIds, error) { + store := ctx.KVStore(k.storeKey) + bz := store.Get(key(time)) + if bz == nil { + return types.ConsumerIds{}, nil + } + + var consumerIds types.ConsumerIds + + if err := consumerIds.Unmarshal(bz); err != nil { + return types.ConsumerIds{}, fmt.Errorf("failed to unmarshal consumer ids: %w", err) + } + return consumerIds, nil +} + +// appendConsumerIdOnTime appends the consumer id on all the other consumer ids under `key(time)` +func (k Keeper) appendConsumerIdOnTime(ctx sdk.Context, consumerId string, key func(time.Time) []byte, time time.Time) error { + store := ctx.KVStore(k.storeKey) + + consumers, err := k.getConsumerIdsBasedOnTime(ctx, key, time) + if err != nil { + return err + } + + consumersWithAppend := types.ConsumerIds{ + Ids: append(consumers.Ids, consumerId), + } + + bz, err := consumersWithAppend.Marshal() + if err != nil { + return err + } + + store.Set(key(time), bz) + return nil +} + +// removeConsumerIdFromTime removes consumer id stored under `key(time)` +func (k Keeper) removeConsumerIdFromTime(ctx sdk.Context, consumerId string, key func(time.Time) []byte, time time.Time) error { + store := ctx.KVStore(k.storeKey) + + consumers, err := k.getConsumerIdsBasedOnTime(ctx, key, time) + if err != nil { + return err + } + + if len(consumers.Ids) == 0 { + return fmt.Errorf("no consumer ids found for this time: %s", time.String()) + } + + // find the index of the consumer we want to remove + index := -1 + for i := 0; i < len(consumers.Ids); i = i + 1 { + if consumers.Ids[i] == consumerId { + index = i + break + } + } + + if index == -1 { + return fmt.Errorf("failed to find consumer id (%s)", consumerId) + } + + if len(consumers.Ids) == 1 { + store.Delete(key(time)) + return nil + } + + consumersWithRemoval := types.ConsumerIds{ + Ids: append(consumers.Ids[:index], consumers.Ids[index+1:]...), + } + + bz, err := consumersWithRemoval.Marshal() + if err != nil { + return err + } + + store.Set(key(time), bz) + return nil +} + +// GetConsumersToBeLaunched returns all the consumer ids of chains stored under this spawn time +func (k Keeper) GetConsumersToBeLaunched(ctx sdk.Context, spawnTime time.Time) (types.ConsumerIds, error) { + return k.getConsumerIdsBasedOnTime(ctx, types.SpawnTimeToConsumerIdsKey, spawnTime) +} + +// AppendConsumerToBeLaunched appends the provider consumer id for the given spawn time +func (k Keeper) AppendConsumerToBeLaunched(ctx sdk.Context, consumerId string, spawnTime time.Time) error { + return k.appendConsumerIdOnTime(ctx, consumerId, types.SpawnTimeToConsumerIdsKey, spawnTime) +} + +// RemoveConsumerToBeLaunched removes consumer id from if stored for this specific spawn time +func (k Keeper) RemoveConsumerToBeLaunched(ctx sdk.Context, consumerId string, spawnTime time.Time) error { + return k.removeConsumerIdFromTime(ctx, consumerId, types.SpawnTimeToConsumerIdsKey, spawnTime) +} + +// GetConsumersToBeStopped returns all the consumer ids of chains stored under this stop time +func (k Keeper) GetConsumersToBeStopped(ctx sdk.Context, stopTime time.Time) (types.ConsumerIds, error) { + return k.getConsumerIdsBasedOnTime(ctx, types.StopTimeToConsumerIdsKey, stopTime) +} + +// AppendConsumerToBeStopped appends the provider consumer id for the given stop time +func (k Keeper) AppendConsumerToBeStopped(ctx sdk.Context, consumerId string, stopTime time.Time) error { + return k.appendConsumerIdOnTime(ctx, consumerId, types.StopTimeToConsumerIdsKey, stopTime) +} + +// RemoveConsumerToBeStopped removes consumer id from if stored for this specific stop time +func (k Keeper) RemoveConsumerToBeStopped(ctx sdk.Context, consumerId string, stopTime time.Time) error { + return k.removeConsumerIdFromTime(ctx, consumerId, types.StopTimeToConsumerIdsKey, stopTime) +} diff --git a/x/ccv/provider/keeper/consumer_lifecycle_test.go b/x/ccv/provider/keeper/consumer_lifecycle_test.go new file mode 100644 index 0000000000..e05bdb460c --- /dev/null +++ b/x/ccv/provider/keeper/consumer_lifecycle_test.go @@ -0,0 +1,900 @@ +package keeper_test + +import ( + "encoding/json" + "fmt" + "testing" + "time" + + "github.com/golang/mock/gomock" + "github.com/stretchr/testify/require" + + abci "github.com/cometbft/cometbft/abci/types" + + "cosmossdk.io/math" + + sdk "github.com/cosmos/cosmos-sdk/types" + stakingtypes "github.com/cosmos/cosmos-sdk/x/staking/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" + _go "github.com/cosmos/ics23/go" + + cryptotestutil "github.com/cosmos/interchain-security/v5/testutil/crypto" + testkeeper "github.com/cosmos/interchain-security/v5/testutil/keeper" + providerkeeper "github.com/cosmos/interchain-security/v5/x/ccv/provider/keeper" + providertypes "github.com/cosmos/interchain-security/v5/x/ccv/provider/types" + ccvtypes "github.com/cosmos/interchain-security/v5/x/ccv/types" +) + +// TestBeginBlockInit directly tests BeginBlockLaunchConsumers against the spec using helpers defined above. +func TestBeginBlockLaunchConsumers(t *testing.T) { + now := time.Now().UTC() + + keeperParams := testkeeper.NewInMemKeeperParams(t) + providerKeeper, ctx, ctrl, mocks := testkeeper.GetProviderKeeperAndCtx(t, keeperParams) + providerKeeper.SetParams(ctx, providertypes.DefaultParams()) + defer ctrl.Finish() + ctx = ctx.WithBlockTime(now) + + // initialize registration, initialization, and update records + consumerMetadata := []providertypes.ConsumerMetadata{ + { + Name: "name", + Description: "spawn time passed", + }, + { + Name: "title", + Description: "spawn time passed", + }, + { + Name: "title", + Description: "spawn time not passed", + }, + { + Name: "title", + Description: "opt-in chain with at least one validator opted in", + }, + { + Name: "title", + Description: "opt-in chain with no validator opted in", + }, + } + chainIds := []string{"chain0", "chain1", "chain2", "chain3", "chain4"} + + initializationParameters := []providertypes.ConsumerInitializationParameters{ + { + InitialHeight: clienttypes.NewHeight(3, 4), + GenesisHash: []byte{}, + BinaryHash: []byte{}, + SpawnTime: now.Add(-time.Hour * 2).UTC(), + UnbondingPeriod: time.Duration(100000000000), + CcvTimeoutPeriod: time.Duration(100000000000), + TransferTimeoutPeriod: time.Duration(100000000000), + ConsumerRedistributionFraction: "0.75", + BlocksPerDistributionTransmission: 10, + HistoricalEntries: 10000, + DistributionTransmissionChannel: "", + }, + { + InitialHeight: clienttypes.NewHeight(3, 4), + GenesisHash: []byte{}, + BinaryHash: []byte{}, + SpawnTime: now.Add(-time.Hour).UTC(), + UnbondingPeriod: time.Duration(100000000000), + CcvTimeoutPeriod: time.Duration(100000000000), + TransferTimeoutPeriod: time.Duration(100000000000), + ConsumerRedistributionFraction: "0.75", + BlocksPerDistributionTransmission: 10, + HistoricalEntries: 10000, + DistributionTransmissionChannel: "", + }, + { + InitialHeight: clienttypes.NewHeight(3, 4), + GenesisHash: []byte{}, + BinaryHash: []byte{}, + SpawnTime: now.Add(time.Hour).UTC(), + UnbondingPeriod: time.Duration(100000000000), + CcvTimeoutPeriod: time.Duration(100000000000), + TransferTimeoutPeriod: time.Duration(100000000000), + ConsumerRedistributionFraction: "0.75", + BlocksPerDistributionTransmission: 10, + HistoricalEntries: 10000, + DistributionTransmissionChannel: "", + }, + { + InitialHeight: clienttypes.NewHeight(3, 4), + GenesisHash: []byte{}, + BinaryHash: []byte{}, + SpawnTime: now.Add(-time.Hour).UTC(), + UnbondingPeriod: time.Duration(100000000000), + CcvTimeoutPeriod: time.Duration(100000000000), + TransferTimeoutPeriod: time.Duration(100000000000), + ConsumerRedistributionFraction: "0.75", + BlocksPerDistributionTransmission: 10, + HistoricalEntries: 10000, + DistributionTransmissionChannel: "", + }, + { + InitialHeight: clienttypes.NewHeight(3, 4), + GenesisHash: []byte{}, + BinaryHash: []byte{}, + SpawnTime: now.Add(-time.Minute).UTC(), + UnbondingPeriod: time.Duration(100000000000), + CcvTimeoutPeriod: time.Duration(100000000000), + TransferTimeoutPeriod: time.Duration(100000000000), + ConsumerRedistributionFraction: "0.75", + BlocksPerDistributionTransmission: 10, + HistoricalEntries: 10000, + DistributionTransmissionChannel: "", + }, + } + powerShapingParameters := []providertypes.PowerShapingParameters{ + { + Top_N: 50, + ValidatorsPowerCap: 0, + ValidatorSetCap: 0, + Allowlist: []string{}, + Denylist: []string{}, + }, + { + Top_N: 50, + ValidatorsPowerCap: 0, + ValidatorSetCap: 0, + Allowlist: []string{}, + Denylist: []string{}, + }, + { + Top_N: 50, + ValidatorsPowerCap: 0, + ValidatorSetCap: 0, + Allowlist: []string{}, + Denylist: []string{}, + }, + { + Top_N: 0, + ValidatorsPowerCap: 0, + ValidatorSetCap: 0, + Allowlist: []string{}, + Denylist: []string{}, + }, + { + Top_N: 0, + ValidatorsPowerCap: 0, + ValidatorSetCap: 0, + Allowlist: []string{}, + Denylist: []string{}, + }} + + // Expect client creation for only the first, second, and fifth proposals (spawn time already passed and valid) + expectedCalls := testkeeper.GetMocksForCreateConsumerClient(ctx, &mocks, "chain0", clienttypes.NewHeight(3, 4)) + expectedCalls = append(expectedCalls, testkeeper.GetMocksForCreateConsumerClient(ctx, &mocks, "chain1", clienttypes.NewHeight(3, 4))...) + expectedCalls = append(expectedCalls, testkeeper.GetMocksForCreateConsumerClient(ctx, &mocks, "chain3", clienttypes.NewHeight(3, 4))...) + + // The fifth chain would have spawn time passed and hence needs the mocks but the client will not be + // created because `chain4` is an Opt In chain and has no validator opted in + expectedCalls = append(expectedCalls, testkeeper.GetMocksForCreateConsumerClient(ctx, &mocks, "chain4", clienttypes.NewHeight(3, 4))...) + + gomock.InOrder(expectedCalls...) + + // set up all the records + for i, chainId := range chainIds { + providerKeeper.SetConsumerChainId(ctx, fmt.Sprintf("%d", i), chainId) + } + + for i, r := range consumerMetadata { + providerKeeper.SetConsumerMetadata(ctx, fmt.Sprintf("%d", i), r) + } + for i, r := range initializationParameters { + err := providerKeeper.SetConsumerInitializationParameters(ctx, fmt.Sprintf("%d", i), r) + require.NoError(t, err) + // set up the chains in their initialized phase, hence they could launch + providerKeeper.SetConsumerPhase(ctx, fmt.Sprintf("%d", i), providertypes.ConsumerPhase_CONSUMER_PHASE_INITIALIZED) + err = providerKeeper.AppendConsumerToBeLaunched(ctx, fmt.Sprintf("%d", i), r.SpawnTime) + require.NoError(t, err) + } + for i, r := range powerShapingParameters { + err := providerKeeper.SetConsumerPowerShapingParameters(ctx, fmt.Sprintf("%d", i), r) + require.NoError(t, err) + } + + // opt in a sample validator so the chain's proposal can successfully execute + validator := cryptotestutil.NewCryptoIdentityFromIntSeed(0).SDKStakingValidator() + consAddr, _ := validator.GetConsAddr() + testkeeper.SetupMocksForLastBondedValidatorsExpectation(mocks.MockStakingKeeper, 1, []stakingtypes.Validator{validator}, -1) // -1 to allow any number of calls + + valAddr, _ := sdk.ValAddressFromBech32(validator.GetOperator()) + mocks.MockStakingKeeper.EXPECT().GetLastValidatorPower(gomock.Any(), valAddr).Return(int64(1), nil).AnyTimes() + // for the validator, expect a call to GetValidatorByConsAddr with its consensus address + mocks.MockStakingKeeper.EXPECT().GetValidatorByConsAddr(gomock.Any(), consAddr).Return(validator, nil).AnyTimes() + + providerKeeper.SetOptedIn(ctx, "3", providertypes.NewProviderConsAddress(consAddr)) + + providerKeeper.BeginBlockLaunchConsumers(ctx) + + // first chain was successfully launched + phase := providerKeeper.GetConsumerPhase(ctx, "0") + require.Equal(t, providertypes.ConsumerPhase_CONSUMER_PHASE_LAUNCHED, phase) + _, found := providerKeeper.GetConsumerGenesis(ctx, "0") + require.True(t, found) + + // second chain was successfully launched + phase = providerKeeper.GetConsumerPhase(ctx, "1") + require.Equal(t, providertypes.ConsumerPhase_CONSUMER_PHASE_LAUNCHED, phase) + _, found = providerKeeper.GetConsumerGenesis(ctx, "1") + require.True(t, found) + + // third chain was not launched because its spawn time has not passed + phase = providerKeeper.GetConsumerPhase(ctx, "2") + require.Equal(t, providertypes.ConsumerPhase_CONSUMER_PHASE_INITIALIZED, phase) + _, found = providerKeeper.GetConsumerGenesis(ctx, "2") + require.False(t, found) + + // fourth chain corresponds to an Opt-In chain with one opted-in validator and hence the chain gets + // successfully executed + phase = providerKeeper.GetConsumerPhase(ctx, "3") + require.Equal(t, providertypes.ConsumerPhase_CONSUMER_PHASE_LAUNCHED, phase) + _, found = providerKeeper.GetConsumerGenesis(ctx, "3") + require.True(t, found) + + // fifth chain corresponds to an Opt-In chain with no opted-in validators and hence the + // chain launch is NOT successful + phase = providerKeeper.GetConsumerPhase(ctx, "4") + require.Equal(t, providertypes.ConsumerPhase_CONSUMER_PHASE_INITIALIZED, phase) + _, found = providerKeeper.GetConsumerGenesis(ctx, "4") + require.False(t, found) +} + +// TestGetConsumersReadyToLaunch tests that the ready to-be-launched consumer chains are returned +func TestGetConsumersReadyToLaunch(t *testing.T) { + providerKeeper, ctx, ctrl, _ := testkeeper.GetProviderKeeperAndCtx(t, testkeeper.NewInMemKeeperParams(t)) + defer ctrl.Finish() + + // no chains to-be-launched exist + require.Empty(t, providerKeeper.GetConsumersReadyToLaunch(ctx, 5)) + + err := providerKeeper.AppendConsumerToBeLaunched(ctx, "consumerId1", time.Unix(10, 0)) + require.NoError(t, err) + err = providerKeeper.AppendConsumerToBeLaunched(ctx, "consumerId2", time.Unix(20, 0)) + require.NoError(t, err) + err = providerKeeper.AppendConsumerToBeLaunched(ctx, "consumerId3", time.Unix(30, 0)) + require.NoError(t, err) + + // time has not yet reached the spawn time of "consumerId1" + ctx = ctx.WithBlockTime(time.Unix(9, 999999999)) + require.Empty(t, providerKeeper.GetConsumersReadyToLaunch(ctx, 3)) + + // time has reached the spawn time of "consumerId1" + ctx = ctx.WithBlockTime(time.Unix(10, 0)) + require.Equal(t, []string{"consumerId1"}, providerKeeper.GetConsumersReadyToLaunch(ctx, 3)) + + // time has reached the spawn time of "consumerId1" and "consumerId2" + ctx = ctx.WithBlockTime(time.Unix(20, 0)) + require.Equal(t, []string{"consumerId1", "consumerId2"}, providerKeeper.GetConsumersReadyToLaunch(ctx, 3)) + + // time has reached the spawn time of all chains + ctx = ctx.WithBlockTime(time.Unix(30, 0)) + require.Equal(t, []string{"consumerId1", "consumerId2", "consumerId3"}, providerKeeper.GetConsumersReadyToLaunch(ctx, 3)) + + // limit the number of returned consumer chains + require.Equal(t, []string{}, providerKeeper.GetConsumersReadyToLaunch(ctx, 0)) + require.Equal(t, []string{"consumerId1"}, providerKeeper.GetConsumersReadyToLaunch(ctx, 1)) + require.Equal(t, []string{"consumerId1", "consumerId2"}, providerKeeper.GetConsumersReadyToLaunch(ctx, 2)) +} + +// Tests the CreateConsumerClient method against the spec, +// with more granularity than what's covered in TestHandleCreateConsumerChainProposal. +func TestCreateConsumerClient(t *testing.T) { + type testCase struct { + description string + // Any state-mutating setup on keeper and expected mock calls, specific to this test case + setup func(*providerkeeper.Keeper, sdk.Context, *testkeeper.MockedKeepers) + // Whether a client should be created + expClientCreated bool + } + tests := []testCase{ + { + description: "No state mutation, new client should be created", + setup: func(providerKeeper *providerkeeper.Keeper, ctx sdk.Context, mocks *testkeeper.MockedKeepers) { + providerKeeper.SetConsumerPhase(ctx, "0", providertypes.ConsumerPhase_CONSUMER_PHASE_INITIALIZED) + + // Valid client creation is asserted with mock expectations here + testkeeper.SetupMocksForLastBondedValidatorsExpectation(mocks.MockStakingKeeper, 0, []stakingtypes.Validator{}, 1) // returns empty validator set + gomock.InOrder( + testkeeper.GetMocksForCreateConsumerClient(ctx, mocks, "chainID", clienttypes.NewHeight(4, 5))..., + ) + }, + expClientCreated: true, + }, + { + description: "chain for this consumer id has already launched, and hence client was created, NO new one is created", + setup: func(providerKeeper *providerkeeper.Keeper, ctx sdk.Context, mocks *testkeeper.MockedKeepers) { + providerKeeper.SetConsumerPhase(ctx, "0", providertypes.ConsumerPhase_CONSUMER_PHASE_LAUNCHED) + + // Expect none of the client creation related calls to happen + mocks.MockStakingKeeper.EXPECT().UnbondingTime(gomock.Any()).Times(0) + mocks.MockClientKeeper.EXPECT().CreateClient(gomock.Any(), gomock.Any(), gomock.Any()).Times(0) + mocks.MockClientKeeper.EXPECT().GetSelfConsensusState(gomock.Any(), gomock.Any()).Times(0) + testkeeper.SetupMocksForLastBondedValidatorsExpectation(mocks.MockStakingKeeper, 0, []stakingtypes.Validator{}, 0) // returns empty validator set + }, + expClientCreated: false, + }, + } + + for _, tc := range tests { + // Common setup + keeperParams := testkeeper.NewInMemKeeperParams(t) + providerKeeper, ctx, ctrl, mocks := testkeeper.GetProviderKeeperAndCtx(t, keeperParams) + providerKeeper.SetParams(ctx, providertypes.DefaultParams()) + + // Test specific setup + tc.setup(&providerKeeper, ctx, &mocks) + + // Call method with same arbitrary values as defined above in mock expectations. + providerKeeper.SetConsumerChainId(ctx, "0", "chainID") + providerKeeper.SetConsumerMetadata(ctx, "0", testkeeper.GetTestConsumerMetadata()) + providerKeeper.SetConsumerInitializationParameters(ctx, "0", testkeeper.GetTestInitializationParameters()) + providerKeeper.SetConsumerPowerShapingParameters(ctx, "0", testkeeper.GetTestPowerShapingParameters()) + err := providerKeeper.CreateConsumerClient(ctx, "0") + + if tc.expClientCreated { + require.NoError(t, err) + testCreatedConsumerClient(t, ctx, providerKeeper, "0", "clientID") + } else { + require.Error(t, err) + } + + // Assert mock calls from setup functions + ctrl.Finish() + } +} + +// Executes test assertions for a created consumer client. +// +// Note: Separated from TestCreateConsumerClient to also be called from TestCreateConsumerChainProposal. +func testCreatedConsumerClient(t *testing.T, + ctx sdk.Context, providerKeeper providerkeeper.Keeper, consumerId, expectedClientID string, +) { + t.Helper() + // ClientID should be stored. + clientId, found := providerKeeper.GetConsumerClientId(ctx, consumerId) + require.True(t, found, "consumer client not found") + require.Equal(t, expectedClientID, clientId) + + // Only assert that consumer genesis was set, + // more granular tests on consumer genesis should be defined in TestMakeConsumerGenesis + _, ok := providerKeeper.GetConsumerGenesis(ctx, consumerId) + require.True(t, ok) +} + +// TestMakeConsumerGenesis tests the MakeConsumerGenesis keeper method. +// An expected genesis state is hardcoded in json, unmarshaled, and compared +// against an actual consumer genesis state constructed by a provider keeper. +func TestMakeConsumerGenesis(t *testing.T) { + keeperParams := testkeeper.NewInMemKeeperParams(t) + providerKeeper, ctx, ctrl, mocks := testkeeper.GetProviderKeeperAndCtx(t, keeperParams) + moduleParams := providertypes.Params{ + TemplateClient: &ibctmtypes.ClientState{ + TrustLevel: ibctmtypes.DefaultTrustLevel, + MaxClockDrift: 10000000000, + ProofSpecs: []*_go.ProofSpec{ + { + LeafSpec: &_go.LeafOp{ + Hash: _go.HashOp_SHA256, + PrehashKey: _go.HashOp_NO_HASH, + PrehashValue: _go.HashOp_SHA256, + Length: _go.LengthOp_VAR_PROTO, + Prefix: []byte{0x00}, + }, + InnerSpec: &_go.InnerSpec{ + ChildOrder: []int32{0, 1}, + ChildSize: 33, + MinPrefixLength: 4, + MaxPrefixLength: 12, + Hash: _go.HashOp_SHA256, + }, + MaxDepth: 0, + MinDepth: 0, + }, + { + LeafSpec: &_go.LeafOp{ + Hash: _go.HashOp_SHA256, + PrehashKey: _go.HashOp_NO_HASH, + PrehashValue: _go.HashOp_SHA256, + Length: _go.LengthOp_VAR_PROTO, + Prefix: []byte{0x00}, + }, + InnerSpec: &_go.InnerSpec{ + ChildOrder: []int32{0, 1}, + ChildSize: 32, + MinPrefixLength: 1, + MaxPrefixLength: 1, + Hash: _go.HashOp_SHA256, + }, + MaxDepth: 0, + }, + }, + UpgradePath: []string{"upgrade", "upgradedIBCState"}, + AllowUpdateAfterExpiry: true, + AllowUpdateAfterMisbehaviour: true, + }, + // Note these are unused provider parameters for this test, and not actually asserted against + // They must be populated with reasonable values to satisfy SetParams though. + TrustingPeriodFraction: providertypes.DefaultTrustingPeriodFraction, + CcvTimeoutPeriod: ccvtypes.DefaultCCVTimeoutPeriod, + SlashMeterReplenishPeriod: providertypes.DefaultSlashMeterReplenishPeriod, + SlashMeterReplenishFraction: providertypes.DefaultSlashMeterReplenishFraction, + ConsumerRewardDenomRegistrationFee: sdk.Coin{ + Denom: "stake", + Amount: math.NewInt(1000000), + }, + BlocksPerEpoch: 600, + NumberOfEpochsToStartReceivingRewards: 24, + } + providerKeeper.SetParams(ctx, moduleParams) + defer ctrl.Finish() + + // + // Other setup not covered by custom template client state + // + ctx = ctx.WithChainID("testchain1") // consumerId is obtained from ctx + ctx = ctx.WithBlockHeight(5) // RevisionHeight obtained from ctx + testkeeper.SetupMocksForLastBondedValidatorsExpectation(mocks.MockStakingKeeper, 0, []stakingtypes.Validator{}, 1) + gomock.InOrder(testkeeper.GetMocksForMakeConsumerGenesis(ctx, &mocks, 1814400000000000)...) + + // matches params from jsonString + consumerMetadata := providertypes.ConsumerMetadata{ + Name: "name", + Description: "description", + } + + ccvTimeoutPeriod := time.Duration(2419200000000000) + transferTimeoutPeriod := time.Duration(3600000000000) + unbondingPeriod := time.Duration(1728000000000000) + initializationParameters := providertypes.ConsumerInitializationParameters{ + BlocksPerDistributionTransmission: 1000, + CcvTimeoutPeriod: ccvTimeoutPeriod, + TransferTimeoutPeriod: transferTimeoutPeriod, + ConsumerRedistributionFraction: "0.75", + HistoricalEntries: 10000, + UnbondingPeriod: unbondingPeriod, + } + providerKeeper.SetConsumerChainId(ctx, "0", "testchain1") + providerKeeper.SetConsumerMetadata(ctx, "0", consumerMetadata) + providerKeeper.SetConsumerInitializationParameters(ctx, "0", initializationParameters) + providerKeeper.SetConsumerPowerShapingParameters(ctx, "0", providertypes.PowerShapingParameters{}) + + actualGenesis, _, err := providerKeeper.MakeConsumerGenesis(ctx, "0") + require.NoError(t, err) + + // JSON string with tabs, newlines and spaces for readability + jsonString := `{ + "params": { + "enabled": true, + "blocks_per_distribution_transmission": 1000, + "ccv_timeout_period": 2419200000000000, + "transfer_timeout_period": 3600000000000, + "consumer_redistribution_fraction": "0.75", + "historical_entries": 10000, + "unbonding_period": 1728000000000000, + "soft_opt_out_threshold": "0", + "reward_denoms": [], + "provider_reward_denoms": [], + "retry_delay_period": 3600000000000 + }, + "new_chain": true, + "provider" : { + "client_state": { + "chain_id": "testchain1", + "trust_level": { + "numerator": 1, + "denominator": 3 + }, + "trusting_period": 1197504000000000, + "unbonding_period": 1814400000000000, + "max_clock_drift": 10000000000, + "frozen_height": {}, + "latest_height": { + "revision_height": 5 + }, + "proof_specs": [ + { + "leaf_spec": { + "hash": 1, + "prehash_value": 1, + "length": 1, + "prefix": "AA==" + }, + "inner_spec": { + "child_order": [0, 1], + "child_size": 33, + "min_prefix_length": 4, + "max_prefix_length": 12, + "hash": 1 + } + }, + { + "leaf_spec": { + "hash": 1, + "prehash_value": 1, + "length": 1, + "prefix": "AA==" + }, + "inner_spec": { + "child_order": [0, 1], + "child_size": 32, + "min_prefix_length": 1, + "max_prefix_length": 1, + "hash": 1 + } + } + ], + "upgrade_path": ["upgrade", "upgradedIBCState"], + "allow_update_after_expiry": true, + "allow_update_after_misbehaviour": true + }, + "consensus_state": { + "timestamp": "2020-01-02T00:00:10Z", + "root": { + "hash": "LpGpeyQVLUo9HpdsgJr12NP2eCICspcULiWa5u9udOA=" + }, + "next_validators_hash": "E30CE736441FB9101FADDAF7E578ABBE6DFDB67207112350A9A904D554E1F5BE" + }, + "initial_val_set": [ + { + "pub_key": { + "type": "tendermint/PubKeyEd25519", + "value": "dcASx5/LIKZqagJWN0frOlFtcvz91frYmj/zmoZRWro=" + }, + "power": 1 + } + ] + } + }` + + var expectedGenesis ccvtypes.ConsumerGenesisState + err = json.Unmarshal([]byte(jsonString), &expectedGenesis) // ignores tabs, newlines and spaces + require.NoError(t, err) + + // Zeroing out different fields that are challenging to mock + actualGenesis.Provider.InitialValSet = []abci.ValidatorUpdate{} + expectedGenesis.Provider.InitialValSet = []abci.ValidatorUpdate{} + actualGenesis.Provider.ConsensusState = &ibctmtypes.ConsensusState{} + expectedGenesis.Provider.ConsensusState = &ibctmtypes.ConsensusState{} + + require.Equal(t, expectedGenesis, actualGenesis, "consumer chain genesis created incorrectly") +} + +func TestBeginBlockStopConsumers(t *testing.T) { + now := time.Now().UTC() + + keeperParams := testkeeper.NewInMemKeeperParams(t) + providerKeeper, ctx, ctrl, mocks := testkeeper.GetProviderKeeperAndCtx(t, keeperParams) + providerKeeper.SetParams(ctx, providertypes.DefaultParams()) + defer ctrl.Finish() + ctx = ctx.WithBlockTime(now) + + chainIds := []string{"chain1", "chain2", "chain3"} + consumerIds := []string{"consumerId1", "consumerId2", "consumerId3"} + err := providerKeeper.SetConsumerStopTime(ctx, consumerIds[0], now.Add(-time.Hour)) + require.NoError(t, err) + err = providerKeeper.AppendConsumerToBeStopped(ctx, consumerIds[0], now.Add(-time.Hour)) + require.NoError(t, err) + err = providerKeeper.SetConsumerStopTime(ctx, consumerIds[1], now) + require.NoError(t, err) + err = providerKeeper.AppendConsumerToBeStopped(ctx, consumerIds[1], now) + require.NoError(t, err) + err = providerKeeper.SetConsumerStopTime(ctx, consumerIds[2], now.Add(time.Hour)) + require.NoError(t, err) + err = providerKeeper.AppendConsumerToBeStopped(ctx, consumerIds[2], now.Add(time.Hour)) + require.NoError(t, err) + + // + // Mock expectations + // + expectations := []*gomock.Call{} + for i := range consumerIds { + chainId := chainIds[i] + // A consumer chain is setup corresponding to each consumerId, making these mocks necessary + testkeeper.SetupMocksForLastBondedValidatorsExpectation(mocks.MockStakingKeeper, 0, []stakingtypes.Validator{}, 1) + expectations = append(expectations, testkeeper.GetMocksForCreateConsumerClient(ctx, &mocks, + chainId, clienttypes.NewHeight(2, 3))...) + expectations = append(expectations, testkeeper.GetMocksForSetConsumerChain(ctx, &mocks, chainId)...) + } + // Only first two consumer chains should be stopped + expectations = append(expectations, testkeeper.GetMocksForStopConsumerChainWithCloseChannel(ctx, &mocks)...) + expectations = append(expectations, testkeeper.GetMocksForStopConsumerChainWithCloseChannel(ctx, &mocks)...) + + gomock.InOrder(expectations...) + + // + // Remaining setup + // + for i, consumerId := range consumerIds { + // Setup a valid consumer chain for each consumerId + initializationRecord := testkeeper.GetTestInitializationParameters() + initializationRecord.InitialHeight = clienttypes.NewHeight(2, 3) + registrationRecord := testkeeper.GetTestConsumerMetadata() + + providerKeeper.SetConsumerChainId(ctx, consumerId, chainIds[i]) + err = providerKeeper.SetConsumerMetadata(ctx, consumerId, registrationRecord) + require.NoError(t, err) + err = providerKeeper.SetConsumerInitializationParameters(ctx, consumerId, initializationRecord) + require.NoError(t, err) + err = providerKeeper.SetConsumerPowerShapingParameters(ctx, consumerId, testkeeper.GetTestPowerShapingParameters()) + require.NoError(t, err) + providerKeeper.SetConsumerPhase(ctx, consumerId, providertypes.ConsumerPhase_CONSUMER_PHASE_INITIALIZED) + providerKeeper.SetConsumerClientId(ctx, consumerId, "clientID") + + err = providerKeeper.CreateConsumerClient(ctx, consumerId) + require.NoError(t, err) + err = providerKeeper.SetConsumerChain(ctx, "channelID") + require.NoError(t, err) + + // after we have created the consumer client, the chain is considered launched and hence we could later stop the chain + providerKeeper.SetConsumerPhase(ctx, consumerId, providertypes.ConsumerPhase_CONSUMER_PHASE_LAUNCHED) + } + + // + // Test execution + // + + providerKeeper.BeginBlockStopConsumers(ctx) + + // Only the 3rd (final) proposal is still stored as pending + phase := providerKeeper.GetConsumerPhase(ctx, consumerIds[0]) + require.Equal(t, providertypes.ConsumerPhase_CONSUMER_PHASE_STOPPED, phase) + phase = providerKeeper.GetConsumerPhase(ctx, consumerIds[1]) + require.Equal(t, providertypes.ConsumerPhase_CONSUMER_PHASE_STOPPED, phase) + // third chain had a stopTime in the future and hence did not stop + phase = providerKeeper.GetConsumerPhase(ctx, consumerIds[2]) + require.Equal(t, providertypes.ConsumerPhase_CONSUMER_PHASE_LAUNCHED, phase) +} + +func TestGetConsumersReadyToStop(t *testing.T) { + providerKeeper, ctx, ctrl, _ := testkeeper.GetProviderKeeperAndCtx(t, testkeeper.NewInMemKeeperParams(t)) + defer ctrl.Finish() + + // no chains to-be-stopped exist + require.Empty(t, providerKeeper.GetConsumersReadyToStop(ctx, 3)) + + err := providerKeeper.AppendConsumerToBeStopped(ctx, "consumerId1", time.Unix(10, 0)) + require.NoError(t, err) + err = providerKeeper.AppendConsumerToBeStopped(ctx, "consumerId2", time.Unix(20, 0)) + require.NoError(t, err) + err = providerKeeper.AppendConsumerToBeStopped(ctx, "consumerId3", time.Unix(30, 0)) + require.NoError(t, err) + + // time has not yet reached the stop time of "consumerId1" + ctx = ctx.WithBlockTime(time.Unix(9, 999999999)) + require.Empty(t, providerKeeper.GetConsumersReadyToStop(ctx, 3)) + + // time has reached the stop time of "consumerId1" + ctx = ctx.WithBlockTime(time.Unix(10, 0)) + require.Equal(t, []string{"consumerId1"}, providerKeeper.GetConsumersReadyToStop(ctx, 3)) + + // time has reached the stop time of "consumerId1" and "consumerId2" + ctx = ctx.WithBlockTime(time.Unix(20, 0)) + require.Equal(t, []string{"consumerId1", "consumerId2"}, providerKeeper.GetConsumersReadyToStop(ctx, 3)) + + // time has reached the stop time of all chains + ctx = ctx.WithBlockTime(time.Unix(30, 0)) + require.Equal(t, []string{"consumerId1", "consumerId2", "consumerId3"}, providerKeeper.GetConsumersReadyToStop(ctx, 3)) +} + +// Tests the StopConsumerChain method against the spec, +// with more granularity than what's covered in TestHandleLegacyConsumerRemovalProposal, or integration tests. +// See: https://github.com/cosmos/ibc/blob/main/spec/app/ics-028-cross-chain-validation/methods.md#ccv-pcf-stcc1 +// Spec tag: [CCV-PCF-STCC.1] +func TestStopConsumerChain(t *testing.T) { + type testCase struct { + description string + // State-mutating setup specific to this test case + setup func(sdk.Context, *providerkeeper.Keeper, testkeeper.MockedKeepers) + // Whether we should expect the method to return an error + expErr bool + } + + consumerId := "0" + + tests := []testCase{ + { + description: "proposal dropped, client doesn't exist", + setup: func(ctx sdk.Context, providerKeeper *providerkeeper.Keeper, mocks testkeeper.MockedKeepers) { + // No mocks, meaning no external keeper methods are allowed to be called. + }, + expErr: true, + }, + { + description: "valid stop of consumer chain, all mock calls hit", + setup: func(ctx sdk.Context, providerKeeper *providerkeeper.Keeper, mocks testkeeper.MockedKeepers) { + testkeeper.SetupForStoppingConsumerChain(t, ctx, providerKeeper, mocks, consumerId) + + // set consumer minimum equivocation height + providerKeeper.SetEquivocationEvidenceMinHeight(ctx, consumerId, 1) + + // assert mocks for expected calls to `StopConsumerChain` when closing the underlying channel + gomock.InOrder(testkeeper.GetMocksForStopConsumerChainWithCloseChannel(ctx, &mocks)...) + }, + expErr: false, + }, + } + + for _, tc := range tests { + + // Common setup + keeperParams := testkeeper.NewInMemKeeperParams(t) + providerKeeper, ctx, ctrl, mocks := testkeeper.GetProviderKeeperAndCtx(t, keeperParams) + providerKeeper.SetParams(ctx, providertypes.DefaultParams()) + + // Setup specific to test case + tc.setup(ctx, &providerKeeper, mocks) + + err := providerKeeper.StopConsumerChain(ctx, consumerId, true) + + if tc.expErr { + require.Error(t, err, t) + } else { + require.NoError(t, err) + } + + testkeeper.TestProviderStateIsCleanedAfterConsumerChainIsStopped(t, ctx, providerKeeper, consumerId, "channelID") + + ctrl.Finish() + } +} + +// +// Setters and Getters +// + +// TestConsumerStopTime tests the getter, setter, and deletion of the consumer id to stop times methods +func TestConsumerStopTime(t *testing.T) { + providerKeeper, ctx, ctrl, _ := testkeeper.GetProviderKeeperAndCtx(t, testkeeper.NewInMemKeeperParams(t)) + defer ctrl.Finish() + + _, err := providerKeeper.GetConsumerStopTime(ctx, "consumerId") + require.Error(t, err) + + expectedStopTime := time.Unix(1234, 56789) + providerKeeper.SetConsumerStopTime(ctx, "consumerId", expectedStopTime) + actualStopTime, err := providerKeeper.GetConsumerStopTime(ctx, "consumerId") + require.NoError(t, err) + require.Equal(t, actualStopTime, expectedStopTime) + + providerKeeper.DeleteConsumerStopTime(ctx, "consumerId") + _, err = providerKeeper.GetConsumerStopTime(ctx, "consumerId") + require.Error(t, err) +} + +// TestConsumersToBeLaunched tests `AppendConsumerToBeLaunched`, `GetConsumersToBeLaunched`, and `RemoveConsumerToBeLaunched` +func TestConsumersToBeLaunched(t *testing.T) { + providerKeeper, ctx, ctrl, _ := testkeeper.GetProviderKeeperAndCtx(t, testkeeper.NewInMemKeeperParams(t)) + defer ctrl.Finish() + + spawnTime := time.Now() + err := providerKeeper.AppendConsumerToBeLaunched(ctx, "consumerId1", spawnTime) + require.NoError(t, err) + consumers, err := providerKeeper.GetConsumersToBeLaunched(ctx, spawnTime) + require.NoError(t, err) + require.Equal(t, []string{"consumerId1"}, consumers.Ids) + + err = providerKeeper.AppendConsumerToBeLaunched(ctx, "consumerId2", spawnTime) + require.NoError(t, err) + consumers, err = providerKeeper.GetConsumersToBeLaunched(ctx, spawnTime) + require.NoError(t, err) + require.Equal(t, []string{"consumerId1", "consumerId2"}, consumers.Ids) + + err = providerKeeper.AppendConsumerToBeLaunched(ctx, "consumerId3", spawnTime) + require.NoError(t, err) + consumers, err = providerKeeper.GetConsumersToBeLaunched(ctx, spawnTime) + require.NoError(t, err) + require.Equal(t, []string{"consumerId1", "consumerId2", "consumerId3"}, consumers.Ids) + + err = providerKeeper.RemoveConsumerToBeLaunched(ctx, "consumerId2", spawnTime) + require.NoError(t, err) + consumers, err = providerKeeper.GetConsumersToBeLaunched(ctx, spawnTime) + require.NoError(t, err) + require.Equal(t, []string{"consumerId1", "consumerId3"}, consumers.Ids) + + // also add consumer ids under a different spawn time and verify everything under the original spawn time is still there + spawnTimePlusOneHour := spawnTime.Add(time.Hour) + err = providerKeeper.AppendConsumerToBeLaunched(ctx, "consumerId4", spawnTimePlusOneHour) + require.NoError(t, err) + consumers, err = providerKeeper.GetConsumersToBeLaunched(ctx, spawnTimePlusOneHour) + require.NoError(t, err) + require.Equal(t, []string{"consumerId4"}, consumers.Ids) + + consumers, err = providerKeeper.GetConsumersToBeLaunched(ctx, spawnTime) + require.NoError(t, err) + require.Equal(t, []string{"consumerId1", "consumerId3"}, consumers.Ids) + + // start removing all consumers from `spawnTime` + err = providerKeeper.RemoveConsumerToBeLaunched(ctx, "consumerId3", spawnTime) + require.NoError(t, err) + err = providerKeeper.RemoveConsumerToBeLaunched(ctx, "consumerId1", spawnTime) + require.NoError(t, err) + consumers, err = providerKeeper.GetConsumersToBeLaunched(ctx, spawnTime) + require.NoError(t, err) + require.Empty(t, consumers.Ids) + + // remove from `spawnTimePlusOneHour` + err = providerKeeper.RemoveConsumerToBeLaunched(ctx, "consumerId4", spawnTimePlusOneHour) + require.NoError(t, err) + consumers, err = providerKeeper.GetConsumersToBeLaunched(ctx, spawnTimePlusOneHour) + require.NoError(t, err) + require.Empty(t, consumers.Ids) + + // add another consumer for `spawnTime` + err = providerKeeper.AppendConsumerToBeLaunched(ctx, "consumerId5", spawnTime) + require.NoError(t, err) + consumers, err = providerKeeper.GetConsumersToBeLaunched(ctx, spawnTime) + require.NoError(t, err) + require.Equal(t, []string{"consumerId5"}, consumers.Ids) +} + +// TestConsumersToBeStopped tests `AppendConsumerToBeLaunched`, `GetConsumersToBeLaunched`, and `RemoveConsumerToBeLaunched` +func TestConsumersToBeStopped(t *testing.T) { + providerKeeper, ctx, ctrl, _ := testkeeper.GetProviderKeeperAndCtx(t, testkeeper.NewInMemKeeperParams(t)) + defer ctrl.Finish() + + stopTime := time.Now() + err := providerKeeper.AppendConsumerToBeStopped(ctx, "consumerId1", stopTime) + require.NoError(t, err) + consumers, err := providerKeeper.GetConsumersToBeStopped(ctx, stopTime) + require.NoError(t, err) + require.Equal(t, []string{"consumerId1"}, consumers.Ids) + + err = providerKeeper.AppendConsumerToBeStopped(ctx, "consumerId2", stopTime) + require.NoError(t, err) + consumers, err = providerKeeper.GetConsumersToBeStopped(ctx, stopTime) + require.NoError(t, err) + require.Equal(t, []string{"consumerId1", "consumerId2"}, consumers.Ids) + + err = providerKeeper.AppendConsumerToBeStopped(ctx, "consumerId3", stopTime) + require.NoError(t, err) + consumers, err = providerKeeper.GetConsumersToBeStopped(ctx, stopTime) + require.NoError(t, err) + require.Equal(t, []string{"consumerId1", "consumerId2", "consumerId3"}, consumers.Ids) + + err = providerKeeper.RemoveConsumerToBeStopped(ctx, "consumerId2", stopTime) + require.NoError(t, err) + consumers, err = providerKeeper.GetConsumersToBeStopped(ctx, stopTime) + require.NoError(t, err) + require.Equal(t, []string{"consumerId1", "consumerId3"}, consumers.Ids) + + // also add consumer ids under a different stop time and verify everything under the original stop time is still there + stopTimePlusOneHour := stopTime.Add(time.Hour) + err = providerKeeper.AppendConsumerToBeStopped(ctx, "consumerId4", stopTimePlusOneHour) + require.NoError(t, err) + consumers, err = providerKeeper.GetConsumersToBeStopped(ctx, stopTimePlusOneHour) + require.NoError(t, err) + require.Equal(t, []string{"consumerId4"}, consumers.Ids) + + consumers, err = providerKeeper.GetConsumersToBeStopped(ctx, stopTime) + require.NoError(t, err) + require.Equal(t, []string{"consumerId1", "consumerId3"}, consumers.Ids) + + // start removing all consumers from `stopTime` + err = providerKeeper.RemoveConsumerToBeStopped(ctx, "consumerId3", stopTime) + require.NoError(t, err) + err = providerKeeper.RemoveConsumerToBeStopped(ctx, "consumerId1", stopTime) + require.NoError(t, err) + consumers, err = providerKeeper.GetConsumersToBeStopped(ctx, stopTime) + require.NoError(t, err) + require.Empty(t, consumers.Ids) + + // remove from `stopTimePlusOneHour` + err = providerKeeper.RemoveConsumerToBeStopped(ctx, "consumerId4", stopTimePlusOneHour) + require.NoError(t, err) + consumers, err = providerKeeper.GetConsumersToBeStopped(ctx, stopTimePlusOneHour) + require.NoError(t, err) + require.Empty(t, consumers.Ids) + + // add another consumer for `stopTime` + err = providerKeeper.AppendConsumerToBeStopped(ctx, "consumerId5", stopTime) + require.NoError(t, err) + consumers, err = providerKeeper.GetConsumersToBeStopped(ctx, stopTime) + require.NoError(t, err) + require.Equal(t, []string{"consumerId5"}, consumers.Ids) +} diff --git a/x/ccv/provider/keeper/permissionless.go b/x/ccv/provider/keeper/permissionless.go index c530023950..caa89f75c7 100644 --- a/x/ccv/provider/keeper/permissionless.go +++ b/x/ccv/provider/keeper/permissionless.go @@ -6,8 +6,6 @@ import ( "strconv" "time" - errorsmod "cosmossdk.io/errors" - storetypes "cosmossdk.io/store/types" sdk "github.com/cosmos/cosmos-sdk/types" govv1 "github.com/cosmos/cosmos-sdk/x/gov/types/v1" "github.com/cosmos/interchain-security/v5/x/ccv/provider/types" @@ -211,149 +209,6 @@ func (k Keeper) IsConsumerActive(ctx sdk.Context, consumerId string) bool { phase == types.ConsumerPhase_CONSUMER_PHASE_LAUNCHED } -// GetConsumerStopTime returns the stop time associated with the to-be-stopped chain with consumer id -func (k Keeper) GetConsumerStopTime(ctx sdk.Context, consumerId string) (time.Time, error) { - store := ctx.KVStore(k.storeKey) - buf := store.Get(types.ConsumerIdToStopTimeKey(consumerId)) - if buf == nil { - return time.Time{}, fmt.Errorf("failed to retrieve stop time for consumer id (%s)", consumerId) - } - var time time.Time - if err := time.UnmarshalBinary(buf); err != nil { - return time, fmt.Errorf("failed to unmarshal stop time for consumer id (%s): %w", consumerId, err) - } - return time, nil -} - -// SetConsumerStopTime sets the stop time associated with this consumer id -func (k Keeper) SetConsumerStopTime(ctx sdk.Context, consumerId string, stopTime time.Time) error { - store := ctx.KVStore(k.storeKey) - buf, err := stopTime.MarshalBinary() - if err != nil { - return fmt.Errorf("failed to marshal stop time (%+v) for consumer id (%s): %w", stopTime, consumerId, err) - } - store.Set(types.ConsumerIdToStopTimeKey(consumerId), buf) - return nil -} - -// DeleteConsumerStopTime deletes the stop time associated with this consumer id -func (k Keeper) DeleteConsumerStopTime(ctx sdk.Context, consumerId string) { - store := ctx.KVStore(k.storeKey) - store.Delete(types.ConsumerIdToStopTimeKey(consumerId)) -} - -// getConsumerIdsBasedOnTime returns all the consumer ids stored under this specific `key(time)` -func (k Keeper) getConsumerIdsBasedOnTime(ctx sdk.Context, key func(time.Time) []byte, time time.Time) (types.ConsumerIds, error) { - store := ctx.KVStore(k.storeKey) - bz := store.Get(key(time)) - if bz == nil { - return types.ConsumerIds{}, nil - } - - var consumerIds types.ConsumerIds - - if err := consumerIds.Unmarshal(bz); err != nil { - return types.ConsumerIds{}, fmt.Errorf("failed to unmarshal consumer ids: %w", err) - } - return consumerIds, nil -} - -// appendConsumerIdOnTime appends the consumer id on all the other consumer ids under `key(time)` -func (k Keeper) appendConsumerIdOnTime(ctx sdk.Context, consumerId string, key func(time.Time) []byte, time time.Time) error { - store := ctx.KVStore(k.storeKey) - - consumers, err := k.getConsumerIdsBasedOnTime(ctx, key, time) - if err != nil { - return err - } - - consumersWithAppend := types.ConsumerIds{ - Ids: append(consumers.Ids, consumerId), - } - - bz, err := consumersWithAppend.Marshal() - if err != nil { - return err - } - - store.Set(key(time), bz) - return nil -} - -// removeConsumerIdFromTime removes consumer id stored under `key(time)` -func (k Keeper) removeConsumerIdFromTime(ctx sdk.Context, consumerId string, key func(time.Time) []byte, time time.Time) error { - store := ctx.KVStore(k.storeKey) - - consumers, err := k.getConsumerIdsBasedOnTime(ctx, key, time) - if err != nil { - return err - } - - if len(consumers.Ids) == 0 { - return fmt.Errorf("no consumer ids found for this time: %s", time.String()) - } - - // find the index of the consumer we want to remove - index := -1 - for i := 0; i < len(consumers.Ids); i = i + 1 { - if consumers.Ids[i] == consumerId { - index = i - break - } - } - - if index == -1 { - return fmt.Errorf("failed to find consumer id (%s)", consumerId) - } - - if len(consumers.Ids) == 1 { - store.Delete(key(time)) - return nil - } - - consumersWithRemoval := types.ConsumerIds{ - Ids: append(consumers.Ids[:index], consumers.Ids[index+1:]...), - } - - bz, err := consumersWithRemoval.Marshal() - if err != nil { - return err - } - - store.Set(key(time), bz) - return nil -} - -// GetConsumersToBeLaunched returns all the consumer ids of chains stored under this spawn time -func (k Keeper) GetConsumersToBeLaunched(ctx sdk.Context, spawnTime time.Time) (types.ConsumerIds, error) { - return k.getConsumerIdsBasedOnTime(ctx, types.SpawnTimeToConsumerIdsKey, spawnTime) -} - -// AppendConsumerToBeLaunched appends the provider consumer id for the given spawn time -func (k Keeper) AppendConsumerToBeLaunched(ctx sdk.Context, consumerId string, spawnTime time.Time) error { - return k.appendConsumerIdOnTime(ctx, consumerId, types.SpawnTimeToConsumerIdsKey, spawnTime) -} - -// RemoveConsumerToBeLaunched removes consumer id from if stored for this specific spawn time -func (k Keeper) RemoveConsumerToBeLaunched(ctx sdk.Context, consumerId string, spawnTime time.Time) error { - return k.removeConsumerIdFromTime(ctx, consumerId, types.SpawnTimeToConsumerIdsKey, spawnTime) -} - -// GetConsumersToBeStopped returns all the consumer ids of chains stored under this stop time -func (k Keeper) GetConsumersToBeStopped(ctx sdk.Context, stopTime time.Time) (types.ConsumerIds, error) { - return k.getConsumerIdsBasedOnTime(ctx, types.StopTimeToConsumerIdsKey, stopTime) -} - -// AppendConsumerToBeStopped appends the provider consumer id for the given stop time -func (k Keeper) AppendConsumerToBeStopped(ctx sdk.Context, consumerId string, stopTime time.Time) error { - return k.appendConsumerIdOnTime(ctx, consumerId, types.StopTimeToConsumerIdsKey, stopTime) -} - -// RemoveConsumerToBeStopped removes consumer id from if stored for this specific stop time -func (k Keeper) RemoveConsumerToBeStopped(ctx sdk.Context, consumerId string, stopTime time.Time) error { - return k.removeConsumerIdFromTime(ctx, consumerId, types.StopTimeToConsumerIdsKey, stopTime) -} - // GetOptedInConsumerIds returns all the consumer ids where the given validator is opted in func (k Keeper) GetOptedInConsumerIds(ctx sdk.Context, providerAddr types.ProviderConsAddress) (types.ConsumerIds, error) { store := ctx.KVStore(k.storeKey) @@ -437,111 +292,6 @@ func (k Keeper) RemoveOptedInConsumerId(ctx sdk.Context, providerAddr types.Prov return nil } -// GetConsumersReadyToLaunch returns the consumer ids of the pending initialized consumer chains -// that are ready to launch, i.e., consumer clients to be created. -func (k Keeper) GetConsumersReadyToLaunch(ctx sdk.Context, limit uint32) []string { - store := ctx.KVStore(k.storeKey) - - spawnTimeToConsumerIdsKeyPrefix := types.SpawnTimeToConsumerIdsKeyPrefix() - iterator := storetypes.KVStorePrefixIterator(store, []byte{spawnTimeToConsumerIdsKeyPrefix}) - defer iterator.Close() - - result := []string{} - for ; iterator.Valid(); iterator.Next() { - spawnTime, err := types.ParseTime(spawnTimeToConsumerIdsKeyPrefix, iterator.Key()) - if err != nil { - k.Logger(ctx).Error("failed to parse spawn time", - "error", err) - continue - } - if spawnTime.After(ctx.BlockTime()) { - return result - } - - // if current block time is equal to or after spawnTime, and the chain is initialized, we can launch the chain - consumerIds, err := k.GetConsumersToBeLaunched(ctx, spawnTime) - if err != nil { - k.Logger(ctx).Error("failed to retrieve consumers to launch", - "spawn time", spawnTime, - "error", err) - continue - } - if len(result)+len(consumerIds.Ids) >= int(limit) { - remainingConsumerIds := len(result) + len(consumerIds.Ids) - int(limit) - if len(consumerIds.Ids[:len(consumerIds.Ids)-remainingConsumerIds]) == 0 { - return result - } - return append(result, consumerIds.Ids[:len(consumerIds.Ids)-remainingConsumerIds]...) - } else { - result = append(result, consumerIds.Ids...) - } - } - - return result -} - -// GetConsumersReadyToStop returns the consumer ids of the pending launched consumer chains -// that are ready to stop -func (k Keeper) GetConsumersReadyToStop(ctx sdk.Context, limit uint32) []string { - store := ctx.KVStore(k.storeKey) - - stopTimeToConsumerIdsKeyPrefix := types.StopTimeToConsumerIdsKeyPrefix() - iterator := storetypes.KVStorePrefixIterator(store, []byte{stopTimeToConsumerIdsKeyPrefix}) - defer iterator.Close() - - result := []string{} - for ; iterator.Valid(); iterator.Next() { - stopTime, err := types.ParseTime(stopTimeToConsumerIdsKeyPrefix, iterator.Key()) - if err != nil { - k.Logger(ctx).Error("failed to parse stop time", - "error", err) - continue - } - if stopTime.After(ctx.BlockTime()) { - return result - } - - consumers, err := k.GetConsumersToBeStopped(ctx, stopTime) - if err != nil { - k.Logger(ctx).Error("failed to retrieve consumers to stop", - "stop time", stopTime, - "error", err) - continue - } - if len(result)+len(consumers.Ids) >= int(limit) { - remainingConsumerIds := len(result) + len(consumers.Ids) - int(limit) - if len(consumers.Ids[:len(consumers.Ids)-remainingConsumerIds]) == 0 { - return result - } - return append(result, consumers.Ids[:len(consumers.Ids)-remainingConsumerIds]...) - } else { - result = append(result, consumers.Ids...) - } - } - - return result -} - -// LaunchConsumer launches the chain with the provided consumer id by creating the consumer client and the respective -// consumer genesis file -func (k Keeper) LaunchConsumer(ctx sdk.Context, consumerId string) error { - err := k.CreateConsumerClient(ctx, consumerId) - if err != nil { - return err - } - - consumerGenesis, found := k.GetConsumerGenesis(ctx, consumerId) - if !found { - return errorsmod.Wrapf(types.ErrNoConsumerGenesis, "consumer genesis could not be found for consumer id: %s", consumerId) - } - - if len(consumerGenesis.Provider.InitialValSet) == 0 { - return errorsmod.Wrapf(types.ErrInvalidConsumerGenesis, "consumer genesis initial validator set is empty - no validators opted in consumer id: %s", consumerId) - } - - return nil -} - // UpdateAllowlist populates the allowlist store for the consumer chain with this consumer id func (k Keeper) UpdateAllowlist(ctx sdk.Context, consumerId string, allowlist []string) { k.DeleteAllowlist(ctx, consumerId) diff --git a/x/ccv/provider/keeper/permissionless_test.go b/x/ccv/provider/keeper/permissionless_test.go index 71fcc8c51d..2b0534b8a5 100644 --- a/x/ccv/provider/keeper/permissionless_test.go +++ b/x/ccv/provider/keeper/permissionless_test.go @@ -244,157 +244,6 @@ func TestConsumerPhase(t *testing.T) { require.Equal(t, providertypes.ConsumerPhase_CONSUMER_PHASE_LAUNCHED, phase) } -// TestConsumerStopTime tests the getter, setter, and deletion of the consumer id to stop times methods -func TestConsumerStopTime(t *testing.T) { - providerKeeper, ctx, ctrl, _ := testkeeper.GetProviderKeeperAndCtx(t, testkeeper.NewInMemKeeperParams(t)) - defer ctrl.Finish() - - _, err := providerKeeper.GetConsumerStopTime(ctx, "consumerId") - require.Error(t, err) - - expectedStopTime := time.Unix(1234, 56789) - providerKeeper.SetConsumerStopTime(ctx, "consumerId", expectedStopTime) - actualStopTime, err := providerKeeper.GetConsumerStopTime(ctx, "consumerId") - require.NoError(t, err) - require.Equal(t, actualStopTime, expectedStopTime) - - providerKeeper.DeleteConsumerStopTime(ctx, "consumerId") - _, err = providerKeeper.GetConsumerStopTime(ctx, "consumerId") - require.Error(t, err) -} - -// TestConsumersToBeLaunched tests `AppendConsumerToBeLaunched`, `GetConsumersToBeLaunched`, and `RemoveConsumerToBeLaunched` -func TestConsumersToBeLaunched(t *testing.T) { - providerKeeper, ctx, ctrl, _ := testkeeper.GetProviderKeeperAndCtx(t, testkeeper.NewInMemKeeperParams(t)) - defer ctrl.Finish() - - spawnTime := time.Now() - err := providerKeeper.AppendConsumerToBeLaunched(ctx, "consumerId1", spawnTime) - require.NoError(t, err) - consumers, err := providerKeeper.GetConsumersToBeLaunched(ctx, spawnTime) - require.NoError(t, err) - require.Equal(t, []string{"consumerId1"}, consumers.Ids) - - err = providerKeeper.AppendConsumerToBeLaunched(ctx, "consumerId2", spawnTime) - require.NoError(t, err) - consumers, err = providerKeeper.GetConsumersToBeLaunched(ctx, spawnTime) - require.NoError(t, err) - require.Equal(t, []string{"consumerId1", "consumerId2"}, consumers.Ids) - - err = providerKeeper.AppendConsumerToBeLaunched(ctx, "consumerId3", spawnTime) - require.NoError(t, err) - consumers, err = providerKeeper.GetConsumersToBeLaunched(ctx, spawnTime) - require.NoError(t, err) - require.Equal(t, []string{"consumerId1", "consumerId2", "consumerId3"}, consumers.Ids) - - err = providerKeeper.RemoveConsumerToBeLaunched(ctx, "consumerId2", spawnTime) - require.NoError(t, err) - consumers, err = providerKeeper.GetConsumersToBeLaunched(ctx, spawnTime) - require.NoError(t, err) - require.Equal(t, []string{"consumerId1", "consumerId3"}, consumers.Ids) - - // also add consumer ids under a different spawn time and verify everything under the original spawn time is still there - spawnTimePlusOneHour := spawnTime.Add(time.Hour) - err = providerKeeper.AppendConsumerToBeLaunched(ctx, "consumerId4", spawnTimePlusOneHour) - require.NoError(t, err) - consumers, err = providerKeeper.GetConsumersToBeLaunched(ctx, spawnTimePlusOneHour) - require.NoError(t, err) - require.Equal(t, []string{"consumerId4"}, consumers.Ids) - - consumers, err = providerKeeper.GetConsumersToBeLaunched(ctx, spawnTime) - require.NoError(t, err) - require.Equal(t, []string{"consumerId1", "consumerId3"}, consumers.Ids) - - // start removing all consumers from `spawnTime` - err = providerKeeper.RemoveConsumerToBeLaunched(ctx, "consumerId3", spawnTime) - require.NoError(t, err) - err = providerKeeper.RemoveConsumerToBeLaunched(ctx, "consumerId1", spawnTime) - require.NoError(t, err) - consumers, err = providerKeeper.GetConsumersToBeLaunched(ctx, spawnTime) - require.NoError(t, err) - require.Empty(t, consumers.Ids) - - // remove from `spawnTimePlusOneHour` - err = providerKeeper.RemoveConsumerToBeLaunched(ctx, "consumerId4", spawnTimePlusOneHour) - require.NoError(t, err) - consumers, err = providerKeeper.GetConsumersToBeLaunched(ctx, spawnTimePlusOneHour) - require.NoError(t, err) - require.Empty(t, consumers.Ids) - - // add another consumer for `spawnTime` - err = providerKeeper.AppendConsumerToBeLaunched(ctx, "consumerId5", spawnTime) - require.NoError(t, err) - consumers, err = providerKeeper.GetConsumersToBeLaunched(ctx, spawnTime) - require.NoError(t, err) - require.Equal(t, []string{"consumerId5"}, consumers.Ids) -} - -// TestConsumersToBeStopped tests `AppendConsumerToBeLaunched`, `GetConsumersToBeLaunched`, and `RemoveConsumerToBeLaunched` -func TestConsumersToBeStopped(t *testing.T) { - providerKeeper, ctx, ctrl, _ := testkeeper.GetProviderKeeperAndCtx(t, testkeeper.NewInMemKeeperParams(t)) - defer ctrl.Finish() - - stopTime := time.Now() - err := providerKeeper.AppendConsumerToBeStopped(ctx, "consumerId1", stopTime) - require.NoError(t, err) - consumers, err := providerKeeper.GetConsumersToBeStopped(ctx, stopTime) - require.NoError(t, err) - require.Equal(t, []string{"consumerId1"}, consumers.Ids) - - err = providerKeeper.AppendConsumerToBeStopped(ctx, "consumerId2", stopTime) - require.NoError(t, err) - consumers, err = providerKeeper.GetConsumersToBeStopped(ctx, stopTime) - require.NoError(t, err) - require.Equal(t, []string{"consumerId1", "consumerId2"}, consumers.Ids) - - err = providerKeeper.AppendConsumerToBeStopped(ctx, "consumerId3", stopTime) - require.NoError(t, err) - consumers, err = providerKeeper.GetConsumersToBeStopped(ctx, stopTime) - require.NoError(t, err) - require.Equal(t, []string{"consumerId1", "consumerId2", "consumerId3"}, consumers.Ids) - - err = providerKeeper.RemoveConsumerToBeStopped(ctx, "consumerId2", stopTime) - require.NoError(t, err) - consumers, err = providerKeeper.GetConsumersToBeStopped(ctx, stopTime) - require.NoError(t, err) - require.Equal(t, []string{"consumerId1", "consumerId3"}, consumers.Ids) - - // also add consumer ids under a different stop time and verify everything under the original stop time is still there - stopTimePlusOneHour := stopTime.Add(time.Hour) - err = providerKeeper.AppendConsumerToBeStopped(ctx, "consumerId4", stopTimePlusOneHour) - require.NoError(t, err) - consumers, err = providerKeeper.GetConsumersToBeStopped(ctx, stopTimePlusOneHour) - require.NoError(t, err) - require.Equal(t, []string{"consumerId4"}, consumers.Ids) - - consumers, err = providerKeeper.GetConsumersToBeStopped(ctx, stopTime) - require.NoError(t, err) - require.Equal(t, []string{"consumerId1", "consumerId3"}, consumers.Ids) - - // start removing all consumers from `stopTime` - err = providerKeeper.RemoveConsumerToBeStopped(ctx, "consumerId3", stopTime) - require.NoError(t, err) - err = providerKeeper.RemoveConsumerToBeStopped(ctx, "consumerId1", stopTime) - require.NoError(t, err) - consumers, err = providerKeeper.GetConsumersToBeStopped(ctx, stopTime) - require.NoError(t, err) - require.Empty(t, consumers.Ids) - - // remove from `stopTimePlusOneHour` - err = providerKeeper.RemoveConsumerToBeStopped(ctx, "consumerId4", stopTimePlusOneHour) - require.NoError(t, err) - consumers, err = providerKeeper.GetConsumersToBeStopped(ctx, stopTimePlusOneHour) - require.NoError(t, err) - require.Empty(t, consumers.Ids) - - // add another consumer for `stopTime` - err = providerKeeper.AppendConsumerToBeStopped(ctx, "consumerId5", stopTime) - require.NoError(t, err) - consumers, err = providerKeeper.GetConsumersToBeStopped(ctx, stopTime) - require.NoError(t, err) - require.Equal(t, []string{"consumerId5"}, consumers.Ids) -} - // TestOptedInConsumerIds tests the `GetOptedInConsumerIds`, `AppendOptedInConsumerId`, and `RemoveOptedInConsumerId` methods func TestGetOptedInConsumerIds(t *testing.T) { providerKeeper, ctx, ctrl, _ := testkeeper.GetProviderKeeperAndCtx(t, testkeeper.NewInMemKeeperParams(t)) @@ -449,74 +298,6 @@ func TestGetOptedInConsumerIds(t *testing.T) { require.Empty(t, consumers) } -// TestGetConsumersReadyToLaunch tests that the ready to-be-launched consumer chains are returned -func TestGetConsumersReadyToLaunch(t *testing.T) { - providerKeeper, ctx, ctrl, _ := testkeeper.GetProviderKeeperAndCtx(t, testkeeper.NewInMemKeeperParams(t)) - defer ctrl.Finish() - - // no chains to-be-launched exist - require.Empty(t, providerKeeper.GetConsumersReadyToLaunch(ctx, 5)) - - err := providerKeeper.AppendConsumerToBeLaunched(ctx, "consumerId1", time.Unix(10, 0)) - require.NoError(t, err) - err = providerKeeper.AppendConsumerToBeLaunched(ctx, "consumerId2", time.Unix(20, 0)) - require.NoError(t, err) - err = providerKeeper.AppendConsumerToBeLaunched(ctx, "consumerId3", time.Unix(30, 0)) - require.NoError(t, err) - - // time has not yet reached the spawn time of "consumerId1" - ctx = ctx.WithBlockTime(time.Unix(9, 999999999)) - require.Empty(t, providerKeeper.GetConsumersReadyToLaunch(ctx, 3)) - - // time has reached the spawn time of "consumerId1" - ctx = ctx.WithBlockTime(time.Unix(10, 0)) - require.Equal(t, []string{"consumerId1"}, providerKeeper.GetConsumersReadyToLaunch(ctx, 3)) - - // time has reached the spawn time of "consumerId1" and "consumerId2" - ctx = ctx.WithBlockTime(time.Unix(20, 0)) - require.Equal(t, []string{"consumerId1", "consumerId2"}, providerKeeper.GetConsumersReadyToLaunch(ctx, 3)) - - // time has reached the spawn time of all chains - ctx = ctx.WithBlockTime(time.Unix(30, 0)) - require.Equal(t, []string{"consumerId1", "consumerId2", "consumerId3"}, providerKeeper.GetConsumersReadyToLaunch(ctx, 3)) - - // limit the number of returned consumer chains - require.Equal(t, []string{}, providerKeeper.GetConsumersReadyToLaunch(ctx, 0)) - require.Equal(t, []string{"consumerId1"}, providerKeeper.GetConsumersReadyToLaunch(ctx, 1)) - require.Equal(t, []string{"consumerId1", "consumerId2"}, providerKeeper.GetConsumersReadyToLaunch(ctx, 2)) -} - -func TestGetConsumersReadyToStop(t *testing.T) { - providerKeeper, ctx, ctrl, _ := testkeeper.GetProviderKeeperAndCtx(t, testkeeper.NewInMemKeeperParams(t)) - defer ctrl.Finish() - - // no chains to-be-stopped exist - require.Empty(t, providerKeeper.GetConsumersReadyToStop(ctx, 3)) - - err := providerKeeper.AppendConsumerToBeStopped(ctx, "consumerId1", time.Unix(10, 0)) - require.NoError(t, err) - err = providerKeeper.AppendConsumerToBeStopped(ctx, "consumerId2", time.Unix(20, 0)) - require.NoError(t, err) - err = providerKeeper.AppendConsumerToBeStopped(ctx, "consumerId3", time.Unix(30, 0)) - require.NoError(t, err) - - // time has not yet reached the stop time of "consumerId1" - ctx = ctx.WithBlockTime(time.Unix(9, 999999999)) - require.Empty(t, providerKeeper.GetConsumersReadyToStop(ctx, 3)) - - // time has reached the stop time of "consumerId1" - ctx = ctx.WithBlockTime(time.Unix(10, 0)) - require.Equal(t, []string{"consumerId1"}, providerKeeper.GetConsumersReadyToStop(ctx, 3)) - - // time has reached the stop time of "consumerId1" and "consumerId2" - ctx = ctx.WithBlockTime(time.Unix(20, 0)) - require.Equal(t, []string{"consumerId1", "consumerId2"}, providerKeeper.GetConsumersReadyToStop(ctx, 3)) - - // time has reached the stop time of all chains - ctx = ctx.WithBlockTime(time.Unix(30, 0)) - require.Equal(t, []string{"consumerId1", "consumerId2", "consumerId3"}, providerKeeper.GetConsumersReadyToStop(ctx, 3)) -} - func TestUpdateAllowlist(t *testing.T) { providerKeeper, ctx, ctrl, _ := testkeeper.GetProviderKeeperAndCtx(t, testkeeper.NewInMemKeeperParams(t)) defer ctrl.Finish() diff --git a/x/ccv/provider/keeper/proposal.go b/x/ccv/provider/keeper/proposal.go index 6d773a22e0..358aee2936 100644 --- a/x/ccv/provider/keeper/proposal.go +++ b/x/ccv/provider/keeper/proposal.go @@ -1,24 +1,9 @@ package keeper import ( - "fmt" - - stakingtypes "github.com/cosmos/cosmos-sdk/x/staking/types" - - clienttypes "github.com/cosmos/ibc-go/v8/modules/core/02-client/types" - channeltypes "github.com/cosmos/ibc-go/v8/modules/core/04-channel/types" - commitmenttypes "github.com/cosmos/ibc-go/v8/modules/core/23-commitment/types" - ibctmtypes "github.com/cosmos/ibc-go/v8/modules/light-clients/07-tendermint" - - errorsmod "cosmossdk.io/errors" - sdk "github.com/cosmos/cosmos-sdk/types" - sdkerrors "github.com/cosmos/cosmos-sdk/types/errors" - - tmtypes "github.com/cometbft/cometbft/types" "github.com/cosmos/interchain-security/v5/x/ccv/provider/types" - ccv "github.com/cosmos/interchain-security/v5/x/ccv/types" ) // Wrapper for the new proposal message MsgChangeRewardDenoms @@ -31,338 +16,3 @@ func (k Keeper) HandleConsumerRewardDenomProposal(ctx sdk.Context, proposal *typ return k.HandleLegacyConsumerRewardDenomProposal(ctx, &p) } - -// CreateConsumerClient will create the CCV client for the given consumer chain. The CCV channel must be built -// on top of the CCV client to ensure connection with the right consumer chain. -func (k Keeper) CreateConsumerClient(ctx sdk.Context, consumerId string) error { - initializationRecord, err := k.GetConsumerInitializationParameters(ctx, consumerId) - if err != nil { - return err - } - - phase := k.GetConsumerPhase(ctx, consumerId) - if phase != types.ConsumerPhase_CONSUMER_PHASE_INITIALIZED { - return errorsmod.Wrapf(types.ErrInvalidPhase, - "cannot create client for consumer chain that is not in the Initialized phase but in phase %d: %s", phase, consumerId) - } - - chainId, err := k.GetConsumerChainId(ctx, consumerId) - if err != nil { - return err - } - - // Set minimum height for equivocation evidence from this consumer chain - k.SetEquivocationEvidenceMinHeight(ctx, consumerId, initializationRecord.InitialHeight.RevisionHeight) - - // Consumers start out with the unbonding period from the consumer addition prop - consumerUnbondingPeriod := initializationRecord.UnbondingPeriod - - // Create client state by getting template client from parameters and filling in zeroed fields from proposal. - clientState := k.GetTemplateClient(ctx) - clientState.ChainId = chainId - clientState.LatestHeight = initializationRecord.InitialHeight - - trustPeriod, err := ccv.CalculateTrustPeriod(consumerUnbondingPeriod, k.GetTrustingPeriodFraction(ctx)) - if err != nil { - return err - } - clientState.TrustingPeriod = trustPeriod - clientState.UnbondingPeriod = consumerUnbondingPeriod - - consumerGen, validatorSetHash, err := k.MakeConsumerGenesis(ctx, consumerId) - if err != nil { - return err - } - err = k.SetConsumerGenesis(ctx, consumerId, consumerGen) - if err != nil { - return err - } - - // Create consensus state - consensusState := ibctmtypes.NewConsensusState( - ctx.BlockTime(), - commitmenttypes.NewMerkleRoot([]byte(ibctmtypes.SentinelRoot)), - validatorSetHash, // use the hash of the updated initial valset - ) - - clientID, err := k.clientKeeper.CreateClient(ctx, clientState, consensusState) - if err != nil { - return err - } - k.SetConsumerClientId(ctx, consumerId, clientID) - - k.Logger(ctx).Info("consumer chain launched (client created)", - "consumer id", consumerId, - "client id", clientID, - ) - - ctx.EventManager().EmitEvent( - sdk.NewEvent( - types.EventTypeConsumerClientCreated, - sdk.NewAttribute(sdk.AttributeKeyModule, types.ModuleName), - sdk.NewAttribute(ccv.AttributeChainID, consumerId), - sdk.NewAttribute(clienttypes.AttributeKeyClientID, clientID), - sdk.NewAttribute(types.AttributeInitialHeight, initializationRecord.InitialHeight.String()), - sdk.NewAttribute(types.AttributeTrustingPeriod, clientState.TrustingPeriod.String()), - sdk.NewAttribute(types.AttributeUnbondingPeriod, clientState.UnbondingPeriod.String()), - ), - ) - - return nil -} - -// StopConsumerChain cleans up the states for the given consumer id -func (k Keeper) StopConsumerChain(ctx sdk.Context, consumerId string, closeChan bool) (err error) { - // check that a client for consumerId exists - // TODO (PERMISSIONLESS): change to use phases instead - if _, found := k.GetConsumerClientId(ctx, consumerId); !found { - return errorsmod.Wrap(types.ErrConsumerChainNotFound, - fmt.Sprintf("cannot stop non-existent consumer chain: %s", consumerId)) - } - - // clean up states - k.DeleteConsumerClientId(ctx, consumerId) - k.DeleteConsumerGenesis(ctx, consumerId) - // Note: this call panics if the key assignment state is invalid - k.DeleteKeyAssignments(ctx, consumerId) - k.DeleteMinimumPowerInTopN(ctx, consumerId) - k.DeleteEquivocationEvidenceMinHeight(ctx, consumerId) - - // close channel and delete the mappings between chain ID and channel ID - if channelID, found := k.GetConsumerIdToChannelId(ctx, consumerId); found { - if closeChan { - // Close the channel for the given channel ID on the condition - // that the channel exists and isn't already in the CLOSED state - channel, found := k.channelKeeper.GetChannel(ctx, ccv.ProviderPortID, channelID) - if found && channel.State != channeltypes.CLOSED { - err := k.chanCloseInit(ctx, channelID) - if err != nil { - k.Logger(ctx).Error("channel to consumer chain could not be closed", - "consumerId", consumerId, - "channelID", channelID, - "error", err.Error(), - ) - } - } - } - k.DeleteConsumerIdToChannelId(ctx, consumerId) - k.DeleteChannelIdToConsumerId(ctx, channelID) - } - - // delete consumer commission rate - provAddrs := k.GetAllCommissionRateValidators(ctx, consumerId) - for _, addr := range provAddrs { - k.DeleteConsumerCommissionRate(ctx, consumerId, addr) - } - - k.DeleteInitChainHeight(ctx, consumerId) - k.DeleteSlashAcks(ctx, consumerId) - k.DeletePendingVSCPackets(ctx, consumerId) - - k.DeleteConsumerMetadata(ctx, consumerId) - k.DeleteConsumerPowerShapingParameters(ctx, consumerId) - k.DeleteConsumerPowerShapingParameters(ctx, consumerId) - k.DeleteAllowlist(ctx, consumerId) - k.DeleteDenylist(ctx, consumerId) - k.DeleteAllOptedIn(ctx, consumerId) - k.DeleteConsumerValSet(ctx, consumerId) - - // TODO (PERMISSIONLESS) add newly-added state to be deleted - - k.Logger(ctx).Info("consumer chain removed from provider", "consumerId", consumerId) - - return nil -} - -// MakeConsumerGenesis returns the created consumer genesis state for consumer chain `consumerId`, -// as well as the validator hash of the initial validator set of the consumer chain -func (k Keeper) MakeConsumerGenesis( - ctx sdk.Context, - consumerId string, -) (gen ccv.ConsumerGenesisState, nextValidatorsHash []byte, err error) { - initializationRecord, err := k.GetConsumerInitializationParameters(ctx, consumerId) - if err != nil { - return gen, nil, errorsmod.Wrapf(ccv.ErrInvalidConsumerState, - "cannot retrieve initialization parameters: %s", err.Error()) - } - powerShapingParameters, err := k.GetConsumerPowerShapingParameters(ctx, consumerId) - if err != nil { - return gen, nil, errorsmod.Wrapf(ccv.ErrInvalidConsumerState, - "cannot retrieve power shaping parameters: %s", err.Error()) - } - - providerUnbondingPeriod, err := k.stakingKeeper.UnbondingTime(ctx) - if err != nil { - return gen, nil, errorsmod.Wrapf(types.ErrNoUnbondingTime, "unbonding time not found: %s", err) - } - height := clienttypes.GetSelfHeight(ctx) - - clientState := k.GetTemplateClient(ctx) - // this is the counter party chain ID for the consumer - clientState.ChainId = ctx.ChainID() - // this is the latest height the client was updated at, i.e., - // the height of the latest consensus state (see below) - clientState.LatestHeight = height - trustPeriod, err := ccv.CalculateTrustPeriod(providerUnbondingPeriod, k.GetTrustingPeriodFraction(ctx)) - if err != nil { - return gen, nil, errorsmod.Wrapf(sdkerrors.ErrInvalidHeight, "error %s calculating trusting_period for: %s", err, height) - } - clientState.TrustingPeriod = trustPeriod - clientState.UnbondingPeriod = providerUnbondingPeriod - - consState, err := k.clientKeeper.GetSelfConsensusState(ctx, height) - if err != nil { - return gen, nil, errorsmod.Wrapf(clienttypes.ErrConsensusStateNotFound, "error %s getting self consensus state for: %s", err, height) - } - - // get the bonded validators from the staking module - bondedValidators, err := k.GetLastBondedValidators(ctx) - if err != nil { - return gen, nil, errorsmod.Wrapf(stakingtypes.ErrNoValidatorFound, "error getting last bonded validators: %s", err) - } - - minPower := int64(0) - if powerShapingParameters.Top_N > 0 { - // get the consensus active validators - // we do not want to base the power calculation for the top N - // on inactive validators, too, since the top N will be a percentage of the active set power - // otherwise, it could be that inactive validators are forced to validate - activeValidators, err := k.GetLastProviderConsensusActiveValidators(ctx) - if err != nil { - return gen, nil, errorsmod.Wrapf(stakingtypes.ErrNoValidatorFound, "error getting last active bonded validators: %s", err) - } - - // in a Top-N chain, we automatically opt in all validators that belong to the top N - minPower, err = k.ComputeMinPowerInTopN(ctx, activeValidators, powerShapingParameters.Top_N) - if err != nil { - return gen, nil, err - } - // log the minimum power in top N - k.Logger(ctx).Info("minimum power in top N at consumer genesis", - "consumerId", consumerId, - "minPower", minPower, - ) - k.OptInTopNValidators(ctx, consumerId, activeValidators, minPower) - k.SetMinimumPowerInTopN(ctx, consumerId, minPower) - } - - // need to use the bondedValidators, not activeValidators, here since the chain might be opt-in and allow inactive vals - nextValidators := k.ComputeNextValidators(ctx, consumerId, bondedValidators, powerShapingParameters, minPower) - k.SetConsumerValSet(ctx, consumerId, nextValidators) - - // get the initial updates with the latest set consumer public keys - initialUpdatesWithConsumerKeys := DiffValidators([]types.ConsensusValidator{}, nextValidators) - - // Get a hash of the consumer validator set from the update with applied consumer assigned keys - updatesAsValSet, err := tmtypes.PB2TM.ValidatorUpdates(initialUpdatesWithConsumerKeys) - if err != nil { - return gen, nil, fmt.Errorf("unable to create validator set from updates computed from key assignment in MakeConsumerGenesis: %s", err) - } - hash := tmtypes.NewValidatorSet(updatesAsValSet).Hash() - - consumerGenesisParams := ccv.NewParams( - true, - initializationRecord.BlocksPerDistributionTransmission, - initializationRecord.DistributionTransmissionChannel, - "", // providerFeePoolAddrStr, - initializationRecord.CcvTimeoutPeriod, - initializationRecord.TransferTimeoutPeriod, - initializationRecord.ConsumerRedistributionFraction, - initializationRecord.HistoricalEntries, - initializationRecord.UnbondingPeriod, - []string{}, - []string{}, - ccv.DefaultRetryDelayPeriod, - ) - - gen = *ccv.NewInitialConsumerGenesisState( - clientState, - consState.(*ibctmtypes.ConsensusState), - initialUpdatesWithConsumerKeys, - consumerGenesisParams, - ) - return gen, hash, nil -} - -// BeginBlockInit iterates over the initialized consumers chains and creates clients for chains -// in which the spawn time has passed -func (k Keeper) BeginBlockInit(ctx sdk.Context) { - // TODO (PERMISSIONLESS): we can parameterize the limit - for _, consumerId := range k.GetConsumersReadyToLaunch(ctx, 200) { - record, err := k.GetConsumerInitializationParameters(ctx, consumerId) - if err != nil { - ctx.Logger().Error("could not retrieve initialization record", - "consumerId", consumerId, - "error", err) - continue - } - // Remove consumer to prevent re-trying launching this chain. - // We would only try to re-launch this chain if a new `MsgUpdateConsumer` message is sent. - err = k.RemoveConsumerToBeLaunched(ctx, consumerId, record.SpawnTime) - if err != nil { - ctx.Logger().Error("could not remove consumer from to-be-launched queue", - "consumerId", consumerId, - "error", err) - continue - } - - cachedCtx, writeFn := ctx.CacheContext() - err = k.LaunchConsumer(cachedCtx, consumerId) - if err != nil { - ctx.Logger().Error("could not launch chain", - "consumerId", consumerId, - "error", err) - continue - } - k.SetConsumerPhase(cachedCtx, consumerId, types.ConsumerPhase_CONSUMER_PHASE_LAUNCHED) - - // the cached context is created with a new EventManager, so we merge the events into the original context - ctx.EventManager().EmitEvents(cachedCtx.EventManager().Events()) - writeFn() - } -} - -// BeginBlockCCR iterates over the pending consumer proposals and stop/removes the chain if the stop time has passed -func (k Keeper) BeginBlockCCR(ctx sdk.Context) { - // TODO (PERMISSIONLESS): parameterize the limit - for _, consumerId := range k.GetConsumersReadyToStop(ctx, 200) { - // stop consumer chain in a cached context to handle errors - cachedCtx, writeFn := ctx.CacheContext() - - stopTime, err := k.GetConsumerStopTime(ctx, consumerId) - if err != nil { - k.Logger(ctx).Error("chain could not be stopped", - "consumerId", consumerId, - "err", err.Error()) - continue - } - - err = k.StopConsumerChain(cachedCtx, consumerId, true) - if err != nil { - k.Logger(ctx).Error("consumer chain could not be stopped", - "consumerId", consumerId, - "err", err.Error()) - continue - } - - k.SetConsumerPhase(cachedCtx, consumerId, types.ConsumerPhase_CONSUMER_PHASE_STOPPED) - - err = k.RemoveConsumerToBeStopped(ctx, consumerId, stopTime) - if err != nil { - ctx.Logger().Error("could not remove consumer from to-be-stopped queue", - "consumerId", consumerId, - "error", err) - continue - } - - // The cached context is created with a new EventManager so we merge the event into the original context - ctx.EventManager().EmitEvents(cachedCtx.EventManager().Events()) - - writeFn() - - k.Logger(ctx).Info("executed consumer removal", - "consumer id", consumerId, - "stop time", stopTime, - ) - } -} diff --git a/x/ccv/provider/keeper/proposal_test.go b/x/ccv/provider/keeper/proposal_test.go index f65fae963a..2779426c61 100644 --- a/x/ccv/provider/keeper/proposal_test.go +++ b/x/ccv/provider/keeper/proposal_test.go @@ -1,183 +1,9 @@ package keeper_test -import ( - "encoding/json" - "fmt" - - "testing" - "time" - - cryptotestutil "github.com/cosmos/interchain-security/v5/testutil/crypto" - - stakingtypes "github.com/cosmos/cosmos-sdk/x/staking/types" - - "cosmossdk.io/math" - clienttypes "github.com/cosmos/ibc-go/v8/modules/core/02-client/types" - ibctmtypes "github.com/cosmos/ibc-go/v8/modules/light-clients/07-tendermint" - _go "github.com/cosmos/ics23/go" - "github.com/golang/mock/gomock" - "github.com/stretchr/testify/require" - - abci "github.com/cometbft/cometbft/abci/types" - sdk "github.com/cosmos/cosmos-sdk/types" - - testkeeper "github.com/cosmos/interchain-security/v5/testutil/keeper" - providerkeeper "github.com/cosmos/interchain-security/v5/x/ccv/provider/keeper" - providertypes "github.com/cosmos/interchain-security/v5/x/ccv/provider/types" - ccvtypes "github.com/cosmos/interchain-security/v5/x/ccv/types" -) - // // Initialization sub-protocol related tests of proposal.go // -// Tests the CreateConsumerClient method against the spec, -// with more granularity than what's covered in TestHandleCreateConsumerChainProposal. -func TestCreateConsumerClient(t *testing.T) { - type testCase struct { - description string - // Any state-mutating setup on keeper and expected mock calls, specific to this test case - setup func(*providerkeeper.Keeper, sdk.Context, *testkeeper.MockedKeepers) - // Whether a client should be created - expClientCreated bool - } - tests := []testCase{ - { - description: "No state mutation, new client should be created", - setup: func(providerKeeper *providerkeeper.Keeper, ctx sdk.Context, mocks *testkeeper.MockedKeepers) { - providerKeeper.SetConsumerPhase(ctx, "0", providertypes.ConsumerPhase_CONSUMER_PHASE_INITIALIZED) - - // Valid client creation is asserted with mock expectations here - testkeeper.SetupMocksForLastBondedValidatorsExpectation(mocks.MockStakingKeeper, 0, []stakingtypes.Validator{}, 1) // returns empty validator set - gomock.InOrder( - testkeeper.GetMocksForCreateConsumerClient(ctx, mocks, "chainID", clienttypes.NewHeight(4, 5))..., - ) - }, - expClientCreated: true, - }, - { - description: "chain for this consumer id has already launched, and hence client was created, NO new one is created", - setup: func(providerKeeper *providerkeeper.Keeper, ctx sdk.Context, mocks *testkeeper.MockedKeepers) { - providerKeeper.SetConsumerPhase(ctx, "0", providertypes.ConsumerPhase_CONSUMER_PHASE_LAUNCHED) - - // Expect none of the client creation related calls to happen - mocks.MockStakingKeeper.EXPECT().UnbondingTime(gomock.Any()).Times(0) - mocks.MockClientKeeper.EXPECT().CreateClient(gomock.Any(), gomock.Any(), gomock.Any()).Times(0) - mocks.MockClientKeeper.EXPECT().GetSelfConsensusState(gomock.Any(), gomock.Any()).Times(0) - testkeeper.SetupMocksForLastBondedValidatorsExpectation(mocks.MockStakingKeeper, 0, []stakingtypes.Validator{}, 0) // returns empty validator set - }, - expClientCreated: false, - }, - } - - for _, tc := range tests { - // Common setup - keeperParams := testkeeper.NewInMemKeeperParams(t) - providerKeeper, ctx, ctrl, mocks := testkeeper.GetProviderKeeperAndCtx(t, keeperParams) - providerKeeper.SetParams(ctx, providertypes.DefaultParams()) - - // Test specific setup - tc.setup(&providerKeeper, ctx, &mocks) - - // Call method with same arbitrary values as defined above in mock expectations. - providerKeeper.SetConsumerChainId(ctx, "0", "chainID") - providerKeeper.SetConsumerMetadata(ctx, "0", testkeeper.GetTestConsumerMetadata()) - providerKeeper.SetConsumerInitializationParameters(ctx, "0", testkeeper.GetTestInitializationParameters()) - providerKeeper.SetConsumerPowerShapingParameters(ctx, "0", testkeeper.GetTestPowerShapingParameters()) - err := providerKeeper.CreateConsumerClient(ctx, "0") - - if tc.expClientCreated { - require.NoError(t, err) - testCreatedConsumerClient(t, ctx, providerKeeper, "0", "clientID") - } else { - require.Error(t, err) - } - - // Assert mock calls from setup functions - ctrl.Finish() - } -} - -// Executes test assertions for a created consumer client. -// -// Note: Separated from TestCreateConsumerClient to also be called from TestCreateConsumerChainProposal. -func testCreatedConsumerClient(t *testing.T, - ctx sdk.Context, providerKeeper providerkeeper.Keeper, consumerId, expectedClientID string, -) { - t.Helper() - // ClientID should be stored. - clientId, found := providerKeeper.GetConsumerClientId(ctx, consumerId) - require.True(t, found, "consumer client not found") - require.Equal(t, expectedClientID, clientId) - - // Only assert that consumer genesis was set, - // more granular tests on consumer genesis should be defined in TestMakeConsumerGenesis - _, ok := providerKeeper.GetConsumerGenesis(ctx, consumerId) - require.True(t, ok) -} - -// Tests the StopConsumerChain method against the spec, -// with more granularity than what's covered in TestHandleLegacyConsumerRemovalProposal, or integration tests. -// See: https://github.com/cosmos/ibc/blob/main/spec/app/ics-028-cross-chain-validation/methods.md#ccv-pcf-stcc1 -// Spec tag: [CCV-PCF-STCC.1] -func TestStopConsumerChain(t *testing.T) { - type testCase struct { - description string - // State-mutating setup specific to this test case - setup func(sdk.Context, *providerkeeper.Keeper, testkeeper.MockedKeepers) - // Whether we should expect the method to return an error - expErr bool - } - - consumerId := "0" - - tests := []testCase{ - { - description: "proposal dropped, client doesn't exist", - setup: func(ctx sdk.Context, providerKeeper *providerkeeper.Keeper, mocks testkeeper.MockedKeepers) { - // No mocks, meaning no external keeper methods are allowed to be called. - }, - expErr: true, - }, - { - description: "valid stop of consumer chain, all mock calls hit", - setup: func(ctx sdk.Context, providerKeeper *providerkeeper.Keeper, mocks testkeeper.MockedKeepers) { - testkeeper.SetupForStoppingConsumerChain(t, ctx, providerKeeper, mocks, consumerId) - - // set consumer minimum equivocation height - providerKeeper.SetEquivocationEvidenceMinHeight(ctx, consumerId, 1) - - // assert mocks for expected calls to `StopConsumerChain` when closing the underlying channel - gomock.InOrder(testkeeper.GetMocksForStopConsumerChainWithCloseChannel(ctx, &mocks)...) - }, - expErr: false, - }, - } - - for _, tc := range tests { - - // Common setup - keeperParams := testkeeper.NewInMemKeeperParams(t) - providerKeeper, ctx, ctrl, mocks := testkeeper.GetProviderKeeperAndCtx(t, keeperParams) - providerKeeper.SetParams(ctx, providertypes.DefaultParams()) - - // Setup specific to test case - tc.setup(ctx, &providerKeeper, mocks) - - err := providerKeeper.StopConsumerChain(ctx, consumerId, true) - - if tc.expErr { - require.Error(t, err, t) - } else { - require.NoError(t, err) - } - - testkeeper.TestProviderStateIsCleanedAfterConsumerChainIsStopped(t, ctx, providerKeeper, consumerId, "channelID") - - ctrl.Finish() - } -} - // TODO (PERMISSIONLESS) // WE DO NOT go by order in permissionless (?) DO WE need to? // TestGetConsumerRemovalPropsToExecute tests that pending consumer removal proposals @@ -252,505 +78,3 @@ func TestStopConsumerChain(t *testing.T) { // require.Equal(t, expectedOrderedProps, propsToExecute) // } //} - -// TestMakeConsumerGenesis tests the MakeConsumerGenesis keeper method. -// An expected genesis state is hardcoded in json, unmarshaled, and compared -// against an actual consumer genesis state constructed by a provider keeper. -func TestMakeConsumerGenesis(t *testing.T) { - keeperParams := testkeeper.NewInMemKeeperParams(t) - providerKeeper, ctx, ctrl, mocks := testkeeper.GetProviderKeeperAndCtx(t, keeperParams) - moduleParams := providertypes.Params{ - TemplateClient: &ibctmtypes.ClientState{ - TrustLevel: ibctmtypes.DefaultTrustLevel, - MaxClockDrift: 10000000000, - ProofSpecs: []*_go.ProofSpec{ - { - LeafSpec: &_go.LeafOp{ - Hash: _go.HashOp_SHA256, - PrehashKey: _go.HashOp_NO_HASH, - PrehashValue: _go.HashOp_SHA256, - Length: _go.LengthOp_VAR_PROTO, - Prefix: []byte{0x00}, - }, - InnerSpec: &_go.InnerSpec{ - ChildOrder: []int32{0, 1}, - ChildSize: 33, - MinPrefixLength: 4, - MaxPrefixLength: 12, - Hash: _go.HashOp_SHA256, - }, - MaxDepth: 0, - MinDepth: 0, - }, - { - LeafSpec: &_go.LeafOp{ - Hash: _go.HashOp_SHA256, - PrehashKey: _go.HashOp_NO_HASH, - PrehashValue: _go.HashOp_SHA256, - Length: _go.LengthOp_VAR_PROTO, - Prefix: []byte{0x00}, - }, - InnerSpec: &_go.InnerSpec{ - ChildOrder: []int32{0, 1}, - ChildSize: 32, - MinPrefixLength: 1, - MaxPrefixLength: 1, - Hash: _go.HashOp_SHA256, - }, - MaxDepth: 0, - }, - }, - UpgradePath: []string{"upgrade", "upgradedIBCState"}, - AllowUpdateAfterExpiry: true, - AllowUpdateAfterMisbehaviour: true, - }, - // Note these are unused provider parameters for this test, and not actually asserted against - // They must be populated with reasonable values to satisfy SetParams though. - TrustingPeriodFraction: providertypes.DefaultTrustingPeriodFraction, - CcvTimeoutPeriod: ccvtypes.DefaultCCVTimeoutPeriod, - SlashMeterReplenishPeriod: providertypes.DefaultSlashMeterReplenishPeriod, - SlashMeterReplenishFraction: providertypes.DefaultSlashMeterReplenishFraction, - ConsumerRewardDenomRegistrationFee: sdk.Coin{ - Denom: "stake", - Amount: math.NewInt(1000000), - }, - BlocksPerEpoch: 600, - NumberOfEpochsToStartReceivingRewards: 24, - } - providerKeeper.SetParams(ctx, moduleParams) - defer ctrl.Finish() - - // - // Other setup not covered by custom template client state - // - ctx = ctx.WithChainID("testchain1") // consumerId is obtained from ctx - ctx = ctx.WithBlockHeight(5) // RevisionHeight obtained from ctx - testkeeper.SetupMocksForLastBondedValidatorsExpectation(mocks.MockStakingKeeper, 0, []stakingtypes.Validator{}, 1) - gomock.InOrder(testkeeper.GetMocksForMakeConsumerGenesis(ctx, &mocks, 1814400000000000)...) - - // matches params from jsonString - consumerMetadata := providertypes.ConsumerMetadata{ - Name: "name", - Description: "description", - } - - ccvTimeoutPeriod := time.Duration(2419200000000000) - transferTimeoutPeriod := time.Duration(3600000000000) - unbondingPeriod := time.Duration(1728000000000000) - initializationParameters := providertypes.ConsumerInitializationParameters{ - BlocksPerDistributionTransmission: 1000, - CcvTimeoutPeriod: ccvTimeoutPeriod, - TransferTimeoutPeriod: transferTimeoutPeriod, - ConsumerRedistributionFraction: "0.75", - HistoricalEntries: 10000, - UnbondingPeriod: unbondingPeriod, - } - providerKeeper.SetConsumerChainId(ctx, "0", "testchain1") - providerKeeper.SetConsumerMetadata(ctx, "0", consumerMetadata) - providerKeeper.SetConsumerInitializationParameters(ctx, "0", initializationParameters) - providerKeeper.SetConsumerPowerShapingParameters(ctx, "0", providertypes.PowerShapingParameters{}) - - actualGenesis, _, err := providerKeeper.MakeConsumerGenesis(ctx, "0") - require.NoError(t, err) - - // JSON string with tabs, newlines and spaces for readability - jsonString := `{ - "params": { - "enabled": true, - "blocks_per_distribution_transmission": 1000, - "ccv_timeout_period": 2419200000000000, - "transfer_timeout_period": 3600000000000, - "consumer_redistribution_fraction": "0.75", - "historical_entries": 10000, - "unbonding_period": 1728000000000000, - "soft_opt_out_threshold": "0", - "reward_denoms": [], - "provider_reward_denoms": [], - "retry_delay_period": 3600000000000 - }, - "new_chain": true, - "provider" : { - "client_state": { - "chain_id": "testchain1", - "trust_level": { - "numerator": 1, - "denominator": 3 - }, - "trusting_period": 1197504000000000, - "unbonding_period": 1814400000000000, - "max_clock_drift": 10000000000, - "frozen_height": {}, - "latest_height": { - "revision_height": 5 - }, - "proof_specs": [ - { - "leaf_spec": { - "hash": 1, - "prehash_value": 1, - "length": 1, - "prefix": "AA==" - }, - "inner_spec": { - "child_order": [0, 1], - "child_size": 33, - "min_prefix_length": 4, - "max_prefix_length": 12, - "hash": 1 - } - }, - { - "leaf_spec": { - "hash": 1, - "prehash_value": 1, - "length": 1, - "prefix": "AA==" - }, - "inner_spec": { - "child_order": [0, 1], - "child_size": 32, - "min_prefix_length": 1, - "max_prefix_length": 1, - "hash": 1 - } - } - ], - "upgrade_path": ["upgrade", "upgradedIBCState"], - "allow_update_after_expiry": true, - "allow_update_after_misbehaviour": true - }, - "consensus_state": { - "timestamp": "2020-01-02T00:00:10Z", - "root": { - "hash": "LpGpeyQVLUo9HpdsgJr12NP2eCICspcULiWa5u9udOA=" - }, - "next_validators_hash": "E30CE736441FB9101FADDAF7E578ABBE6DFDB67207112350A9A904D554E1F5BE" - }, - "initial_val_set": [ - { - "pub_key": { - "type": "tendermint/PubKeyEd25519", - "value": "dcASx5/LIKZqagJWN0frOlFtcvz91frYmj/zmoZRWro=" - }, - "power": 1 - } - ] - } - }` - - var expectedGenesis ccvtypes.ConsumerGenesisState - err = json.Unmarshal([]byte(jsonString), &expectedGenesis) // ignores tabs, newlines and spaces - require.NoError(t, err) - - // Zeroing out different fields that are challenging to mock - actualGenesis.Provider.InitialValSet = []abci.ValidatorUpdate{} - expectedGenesis.Provider.InitialValSet = []abci.ValidatorUpdate{} - actualGenesis.Provider.ConsensusState = &ibctmtypes.ConsensusState{} - expectedGenesis.Provider.ConsensusState = &ibctmtypes.ConsensusState{} - - require.Equal(t, expectedGenesis, actualGenesis, "consumer chain genesis created incorrectly") -} - -// TestBeginBlockInit directly tests BeginBlockInit against the spec using helpers defined above. -func TestBeginBlockInit(t *testing.T) { - now := time.Now().UTC() - - keeperParams := testkeeper.NewInMemKeeperParams(t) - providerKeeper, ctx, ctrl, mocks := testkeeper.GetProviderKeeperAndCtx(t, keeperParams) - providerKeeper.SetParams(ctx, providertypes.DefaultParams()) - defer ctrl.Finish() - ctx = ctx.WithBlockTime(now) - - // initialize registration, initialization, and update records - consumerMetadata := []providertypes.ConsumerMetadata{ - { - Name: "name", - Description: "spawn time passed", - }, - { - Name: "title", - Description: "spawn time passed", - }, - { - Name: "title", - Description: "spawn time not passed", - }, - { - Name: "title", - Description: "opt-in chain with at least one validator opted in", - }, - { - Name: "title", - Description: "opt-in chain with no validator opted in", - }, - } - chainIds := []string{"chain0", "chain1", "chain2", "chain3", "chain4"} - - initializationParameters := []providertypes.ConsumerInitializationParameters{ - { - InitialHeight: clienttypes.NewHeight(3, 4), - GenesisHash: []byte{}, - BinaryHash: []byte{}, - SpawnTime: now.Add(-time.Hour * 2).UTC(), - UnbondingPeriod: time.Duration(100000000000), - CcvTimeoutPeriod: time.Duration(100000000000), - TransferTimeoutPeriod: time.Duration(100000000000), - ConsumerRedistributionFraction: "0.75", - BlocksPerDistributionTransmission: 10, - HistoricalEntries: 10000, - DistributionTransmissionChannel: "", - }, - { - InitialHeight: clienttypes.NewHeight(3, 4), - GenesisHash: []byte{}, - BinaryHash: []byte{}, - SpawnTime: now.Add(-time.Hour).UTC(), - UnbondingPeriod: time.Duration(100000000000), - CcvTimeoutPeriod: time.Duration(100000000000), - TransferTimeoutPeriod: time.Duration(100000000000), - ConsumerRedistributionFraction: "0.75", - BlocksPerDistributionTransmission: 10, - HistoricalEntries: 10000, - DistributionTransmissionChannel: "", - }, - { - InitialHeight: clienttypes.NewHeight(3, 4), - GenesisHash: []byte{}, - BinaryHash: []byte{}, - SpawnTime: now.Add(time.Hour).UTC(), - UnbondingPeriod: time.Duration(100000000000), - CcvTimeoutPeriod: time.Duration(100000000000), - TransferTimeoutPeriod: time.Duration(100000000000), - ConsumerRedistributionFraction: "0.75", - BlocksPerDistributionTransmission: 10, - HistoricalEntries: 10000, - DistributionTransmissionChannel: "", - }, - { - InitialHeight: clienttypes.NewHeight(3, 4), - GenesisHash: []byte{}, - BinaryHash: []byte{}, - SpawnTime: now.Add(-time.Hour).UTC(), - UnbondingPeriod: time.Duration(100000000000), - CcvTimeoutPeriod: time.Duration(100000000000), - TransferTimeoutPeriod: time.Duration(100000000000), - ConsumerRedistributionFraction: "0.75", - BlocksPerDistributionTransmission: 10, - HistoricalEntries: 10000, - DistributionTransmissionChannel: "", - }, - { - InitialHeight: clienttypes.NewHeight(3, 4), - GenesisHash: []byte{}, - BinaryHash: []byte{}, - SpawnTime: now.Add(-time.Minute).UTC(), - UnbondingPeriod: time.Duration(100000000000), - CcvTimeoutPeriod: time.Duration(100000000000), - TransferTimeoutPeriod: time.Duration(100000000000), - ConsumerRedistributionFraction: "0.75", - BlocksPerDistributionTransmission: 10, - HistoricalEntries: 10000, - DistributionTransmissionChannel: "", - }, - } - powerShapingParameters := []providertypes.PowerShapingParameters{ - { - Top_N: 50, - ValidatorsPowerCap: 0, - ValidatorSetCap: 0, - Allowlist: []string{}, - Denylist: []string{}, - }, - { - Top_N: 50, - ValidatorsPowerCap: 0, - ValidatorSetCap: 0, - Allowlist: []string{}, - Denylist: []string{}, - }, - { - Top_N: 50, - ValidatorsPowerCap: 0, - ValidatorSetCap: 0, - Allowlist: []string{}, - Denylist: []string{}, - }, - { - Top_N: 0, - ValidatorsPowerCap: 0, - ValidatorSetCap: 0, - Allowlist: []string{}, - Denylist: []string{}, - }, - { - Top_N: 0, - ValidatorsPowerCap: 0, - ValidatorSetCap: 0, - Allowlist: []string{}, - Denylist: []string{}, - }} - - // Expect client creation for only the first, second, and fifth proposals (spawn time already passed and valid) - expectedCalls := testkeeper.GetMocksForCreateConsumerClient(ctx, &mocks, "chain0", clienttypes.NewHeight(3, 4)) - expectedCalls = append(expectedCalls, testkeeper.GetMocksForCreateConsumerClient(ctx, &mocks, "chain1", clienttypes.NewHeight(3, 4))...) - expectedCalls = append(expectedCalls, testkeeper.GetMocksForCreateConsumerClient(ctx, &mocks, "chain3", clienttypes.NewHeight(3, 4))...) - - // The fifth chain would have spawn time passed and hence needs the mocks but the client will not be - // created because `chain4` is an Opt In chain and has no validator opted in - expectedCalls = append(expectedCalls, testkeeper.GetMocksForCreateConsumerClient(ctx, &mocks, "chain4", clienttypes.NewHeight(3, 4))...) - - gomock.InOrder(expectedCalls...) - - // set up all the records - for i, chainId := range chainIds { - providerKeeper.SetConsumerChainId(ctx, fmt.Sprintf("%d", i), chainId) - } - - for i, r := range consumerMetadata { - providerKeeper.SetConsumerMetadata(ctx, fmt.Sprintf("%d", i), r) - } - for i, r := range initializationParameters { - err := providerKeeper.SetConsumerInitializationParameters(ctx, fmt.Sprintf("%d", i), r) - require.NoError(t, err) - // set up the chains in their initialized phase, hence they could launch - providerKeeper.SetConsumerPhase(ctx, fmt.Sprintf("%d", i), providertypes.ConsumerPhase_CONSUMER_PHASE_INITIALIZED) - err = providerKeeper.AppendConsumerToBeLaunched(ctx, fmt.Sprintf("%d", i), r.SpawnTime) - require.NoError(t, err) - } - for i, r := range powerShapingParameters { - err := providerKeeper.SetConsumerPowerShapingParameters(ctx, fmt.Sprintf("%d", i), r) - require.NoError(t, err) - } - - // opt in a sample validator so the chain's proposal can successfully execute - validator := cryptotestutil.NewCryptoIdentityFromIntSeed(0).SDKStakingValidator() - consAddr, _ := validator.GetConsAddr() - testkeeper.SetupMocksForLastBondedValidatorsExpectation(mocks.MockStakingKeeper, 1, []stakingtypes.Validator{validator}, -1) // -1 to allow any number of calls - - valAddr, _ := sdk.ValAddressFromBech32(validator.GetOperator()) - mocks.MockStakingKeeper.EXPECT().GetLastValidatorPower(gomock.Any(), valAddr).Return(int64(1), nil).AnyTimes() - // for the validator, expect a call to GetValidatorByConsAddr with its consensus address - mocks.MockStakingKeeper.EXPECT().GetValidatorByConsAddr(gomock.Any(), consAddr).Return(validator, nil).AnyTimes() - - providerKeeper.SetOptedIn(ctx, "3", providertypes.NewProviderConsAddress(consAddr)) - - providerKeeper.BeginBlockInit(ctx) - - // first chain was successfully launched - phase := providerKeeper.GetConsumerPhase(ctx, "0") - require.Equal(t, providertypes.ConsumerPhase_CONSUMER_PHASE_LAUNCHED, phase) - _, found := providerKeeper.GetConsumerGenesis(ctx, "0") - require.True(t, found) - - // second chain was successfully launched - phase = providerKeeper.GetConsumerPhase(ctx, "1") - require.Equal(t, providertypes.ConsumerPhase_CONSUMER_PHASE_LAUNCHED, phase) - _, found = providerKeeper.GetConsumerGenesis(ctx, "1") - require.True(t, found) - - // third chain was not launched because its spawn time has not passed - phase = providerKeeper.GetConsumerPhase(ctx, "2") - require.Equal(t, providertypes.ConsumerPhase_CONSUMER_PHASE_INITIALIZED, phase) - _, found = providerKeeper.GetConsumerGenesis(ctx, "2") - require.False(t, found) - - // fourth chain corresponds to an Opt-In chain with one opted-in validator and hence the chain gets - // successfully executed - phase = providerKeeper.GetConsumerPhase(ctx, "3") - require.Equal(t, providertypes.ConsumerPhase_CONSUMER_PHASE_LAUNCHED, phase) - _, found = providerKeeper.GetConsumerGenesis(ctx, "3") - require.True(t, found) - - // fifth chain corresponds to an Opt-In chain with no opted-in validators and hence the - // chain launch is NOT successful - phase = providerKeeper.GetConsumerPhase(ctx, "4") - require.Equal(t, providertypes.ConsumerPhase_CONSUMER_PHASE_INITIALIZED, phase) - _, found = providerKeeper.GetConsumerGenesis(ctx, "4") - require.False(t, found) -} - -func TestBeginBlockCCR(t *testing.T) { - now := time.Now().UTC() - - keeperParams := testkeeper.NewInMemKeeperParams(t) - providerKeeper, ctx, ctrl, mocks := testkeeper.GetProviderKeeperAndCtx(t, keeperParams) - providerKeeper.SetParams(ctx, providertypes.DefaultParams()) - defer ctrl.Finish() - ctx = ctx.WithBlockTime(now) - - chainIds := []string{"chain1", "chain2", "chain3"} - consumerIds := []string{"consumerId1", "consumerId2", "consumerId3"} - err := providerKeeper.SetConsumerStopTime(ctx, consumerIds[0], now.Add(-time.Hour)) - require.NoError(t, err) - err = providerKeeper.AppendConsumerToBeStopped(ctx, consumerIds[0], now.Add(-time.Hour)) - require.NoError(t, err) - err = providerKeeper.SetConsumerStopTime(ctx, consumerIds[1], now) - require.NoError(t, err) - err = providerKeeper.AppendConsumerToBeStopped(ctx, consumerIds[1], now) - require.NoError(t, err) - err = providerKeeper.SetConsumerStopTime(ctx, consumerIds[2], now.Add(time.Hour)) - require.NoError(t, err) - err = providerKeeper.AppendConsumerToBeStopped(ctx, consumerIds[2], now.Add(time.Hour)) - require.NoError(t, err) - - // - // Mock expectations - // - expectations := []*gomock.Call{} - for i := range consumerIds { - chainId := chainIds[i] - // A consumer chain is setup corresponding to each consumerId, making these mocks necessary - testkeeper.SetupMocksForLastBondedValidatorsExpectation(mocks.MockStakingKeeper, 0, []stakingtypes.Validator{}, 1) - expectations = append(expectations, testkeeper.GetMocksForCreateConsumerClient(ctx, &mocks, - chainId, clienttypes.NewHeight(2, 3))...) - expectations = append(expectations, testkeeper.GetMocksForSetConsumerChain(ctx, &mocks, chainId)...) - } - // Only first two consumer chains should be stopped - expectations = append(expectations, testkeeper.GetMocksForStopConsumerChainWithCloseChannel(ctx, &mocks)...) - expectations = append(expectations, testkeeper.GetMocksForStopConsumerChainWithCloseChannel(ctx, &mocks)...) - - gomock.InOrder(expectations...) - - // - // Remaining setup - // - for i, consumerId := range consumerIds { - // Setup a valid consumer chain for each consumerId - initializationRecord := testkeeper.GetTestInitializationParameters() - initializationRecord.InitialHeight = clienttypes.NewHeight(2, 3) - registrationRecord := testkeeper.GetTestConsumerMetadata() - - providerKeeper.SetConsumerChainId(ctx, consumerId, chainIds[i]) - err = providerKeeper.SetConsumerMetadata(ctx, consumerId, registrationRecord) - require.NoError(t, err) - err = providerKeeper.SetConsumerInitializationParameters(ctx, consumerId, initializationRecord) - require.NoError(t, err) - err = providerKeeper.SetConsumerPowerShapingParameters(ctx, consumerId, testkeeper.GetTestPowerShapingParameters()) - require.NoError(t, err) - providerKeeper.SetConsumerPhase(ctx, consumerId, providertypes.ConsumerPhase_CONSUMER_PHASE_INITIALIZED) - providerKeeper.SetConsumerClientId(ctx, consumerId, "clientID") - - err = providerKeeper.CreateConsumerClient(ctx, consumerId) - require.NoError(t, err) - err = providerKeeper.SetConsumerChain(ctx, "channelID") - require.NoError(t, err) - - // after we have created the consumer client, the chain is considered launched and hence we could later stop the chain - providerKeeper.SetConsumerPhase(ctx, consumerId, providertypes.ConsumerPhase_CONSUMER_PHASE_LAUNCHED) - } - - // - // Test execution - // - - providerKeeper.BeginBlockCCR(ctx) - - // Only the 3rd (final) proposal is still stored as pending - phase := providerKeeper.GetConsumerPhase(ctx, consumerIds[0]) - require.Equal(t, providertypes.ConsumerPhase_CONSUMER_PHASE_STOPPED, phase) - phase = providerKeeper.GetConsumerPhase(ctx, consumerIds[1]) - require.Equal(t, providertypes.ConsumerPhase_CONSUMER_PHASE_STOPPED, phase) - // third chain had a stopTime in the future and hence did not stop - phase = providerKeeper.GetConsumerPhase(ctx, consumerIds[2]) - require.Equal(t, providertypes.ConsumerPhase_CONSUMER_PHASE_LAUNCHED, phase) -} diff --git a/x/ccv/provider/module.go b/x/ccv/provider/module.go index 65ced5f37e..30ac49363a 100644 --- a/x/ccv/provider/module.go +++ b/x/ccv/provider/module.go @@ -174,9 +174,9 @@ func (am AppModule) BeginBlock(ctx context.Context) error { sdkCtx := sdk.UnwrapSDKContext(ctx) // Create clients to consumer chains that are due to be spawned - am.keeper.BeginBlockInit(sdkCtx) + am.keeper.BeginBlockLaunchConsumers(sdkCtx) // Stop and remove state for any consumer chains that are due to be stopped - am.keeper.BeginBlockCCR(sdkCtx) + am.keeper.BeginBlockStopConsumers(sdkCtx) // Check for replenishing slash meter before any slash packets are processed for this block am.keeper.BeginBlockCIS(sdkCtx) // BeginBlock logic needed for the Reward Distribution sub-protocol From 10e1f6cdbcd65d7afedaf5145ca2931767ac2005 Mon Sep 17 00:00:00 2001 From: mpoke Date: Thu, 29 Aug 2024 19:53:33 +0200 Subject: [PATCH 05/13] move methods around --- x/ccv/provider/keeper/keeper.go | 50 +++++++++ x/ccv/provider/keeper/keeper_test.go | 102 +++++++++++++++++++ x/ccv/provider/keeper/permissionless.go | 51 ---------- x/ccv/provider/keeper/permissionless_test.go | 101 ------------------ 4 files changed, 152 insertions(+), 152 deletions(-) diff --git a/x/ccv/provider/keeper/keeper.go b/x/ccv/provider/keeper/keeper.go index 7628fe334d..1e11afb8de 100644 --- a/x/ccv/provider/keeper/keeper.go +++ b/x/ccv/provider/keeper/keeper.go @@ -927,6 +927,19 @@ func (k Keeper) IsAllowlistEmpty(ctx sdk.Context, consumerId string) bool { return !iterator.Valid() } +// UpdateAllowlist populates the allowlist store for the consumer chain with this consumer id +func (k Keeper) UpdateAllowlist(ctx sdk.Context, consumerId string, allowlist []string) { + k.DeleteAllowlist(ctx, consumerId) + for _, address := range allowlist { + consAddr, err := sdk.ConsAddressFromBech32(address) + if err != nil { + continue + } + + k.SetAllowlist(ctx, consumerId, types.NewProviderConsAddress(consAddr)) + } +} + // SetDenylist denylists validator with `providerAddr` address on chain `consumerId` func (k Keeper) SetDenylist( ctx sdk.Context, @@ -990,6 +1003,19 @@ func (k Keeper) IsDenylistEmpty(ctx sdk.Context, consumerId string) bool { return !iterator.Valid() } +// UpdateDenylist populates the denylist store for the consumer chain with this consumer id +func (k Keeper) UpdateDenylist(ctx sdk.Context, consumerId string, denylist []string) { + k.DeleteDenylist(ctx, consumerId) + for _, address := range denylist { + consAddr, err := sdk.ConsAddressFromBech32(address) + if err != nil { + continue + } + + k.SetDenylist(ctx, consumerId, types.NewProviderConsAddress(consAddr)) + } +} + // SetMinimumPowerInTopN sets the minimum power required for a validator to be in the top N // for a given consumer chain. func (k Keeper) SetMinimumPowerInTopN( @@ -1029,6 +1055,30 @@ func (k Keeper) DeleteMinimumPowerInTopN( store.Delete(types.MinimumPowerInTopNKey(consumerId)) } +// UpdateMinimumPowerInTopN populates the minimum power in Top N for the consumer chain with this consumer id +func (k Keeper) UpdateMinimumPowerInTopN(ctx sdk.Context, consumerId string, oldTopN uint32, newTopN uint32) error { + // if the top N changes, we need to update the new minimum power in top N + if newTopN != oldTopN { + if newTopN > 0 { + // if the chain receives a non-zero top N value, store the minimum power in the top N + bondedValidators, err := k.GetLastProviderConsensusActiveValidators(ctx) + if err != nil { + return err + } + minPower, err := k.ComputeMinPowerInTopN(ctx, bondedValidators, newTopN) + if err != nil { + return err + } + k.SetMinimumPowerInTopN(ctx, consumerId, minPower) + } else { + // if the chain receives a zero top N value, we delete the min power + k.DeleteMinimumPowerInTopN(ctx, consumerId) + } + } + + return nil +} + func (k Keeper) UnbondingCanComplete(ctx sdk.Context, id uint64) error { return k.stakingKeeper.UnbondingCanComplete(ctx, id) } diff --git a/x/ccv/provider/keeper/keeper_test.go b/x/ccv/provider/keeper/keeper_test.go index 6fb169f92c..a8cb2583bf 100644 --- a/x/ccv/provider/keeper/keeper_test.go +++ b/x/ccv/provider/keeper/keeper_test.go @@ -8,10 +8,12 @@ import ( "cosmossdk.io/math" ibctesting "github.com/cosmos/ibc-go/v8/testing" + "github.com/golang/mock/gomock" "github.com/stretchr/testify/require" cryptocodec "github.com/cosmos/cosmos-sdk/crypto/codec" sdk "github.com/cosmos/cosmos-sdk/types" + stakingtypes "github.com/cosmos/cosmos-sdk/x/staking/types" abci "github.com/cometbft/cometbft/abci/types" tmprotocrypto "github.com/cometbft/cometbft/proto/tendermint/crypto" @@ -19,6 +21,7 @@ import ( cryptotestutil "github.com/cosmos/interchain-security/v5/testutil/crypto" testkeeper "github.com/cosmos/interchain-security/v5/testutil/keeper" "github.com/cosmos/interchain-security/v5/x/ccv/provider/types" + providertypes "github.com/cosmos/interchain-security/v5/x/ccv/provider/types" ccv "github.com/cosmos/interchain-security/v5/x/ccv/types" ) @@ -398,6 +401,25 @@ func TestAllowlist(t *testing.T) { require.True(t, providerKeeper.IsAllowlistEmpty(ctx, chainID)) } +func TestUpdateAllowlist(t *testing.T) { + providerKeeper, ctx, ctrl, _ := testkeeper.GetProviderKeeperAndCtx(t, testkeeper.NewInMemKeeperParams(t)) + defer ctrl.Finish() + + consumerId := "0" + + providerConsAddr1 := "cosmosvalcons1qmq08eruchr5sf5s3rwz7djpr5a25f7xw4mceq" + consAddr1, _ := sdk.ConsAddressFromBech32(providerConsAddr1) + providerConsAddr2 := "cosmosvalcons1nx7n5uh0ztxsynn4sje6eyq2ud6rc6klc96w39" + consAddr2, _ := sdk.ConsAddressFromBech32(providerConsAddr2) + + providerKeeper.UpdateAllowlist(ctx, consumerId, []string{providerConsAddr1, providerConsAddr2}) + + expectedAllowlist := []providertypes.ProviderConsAddress{ + providertypes.NewProviderConsAddress(consAddr1), + providertypes.NewProviderConsAddress(consAddr2)} + require.Equal(t, expectedAllowlist, providerKeeper.GetAllowList(ctx, consumerId)) +} + // TestDenylist tests the `SetDenylist`, `IsDenylisted`, `DeleteDenylist`, and `IsDenylistEmpty` methods func TestDenylist(t *testing.T) { providerKeeper, ctx, ctrl, _ := testkeeper.GetProviderKeeperAndCtx(t, testkeeper.NewInMemKeeperParams(t)) @@ -426,6 +448,25 @@ func TestDenylist(t *testing.T) { require.True(t, providerKeeper.IsDenylistEmpty(ctx, chainID)) } +func TestUpdateDenylist(t *testing.T) { + providerKeeper, ctx, ctrl, _ := testkeeper.GetProviderKeeperAndCtx(t, testkeeper.NewInMemKeeperParams(t)) + defer ctrl.Finish() + + consumerId := "0" + + providerConsAddr1 := "cosmosvalcons1qmq08eruchr5sf5s3rwz7djpr5a25f7xw4mceq" + consAddr1, _ := sdk.ConsAddressFromBech32(providerConsAddr1) + providerConsAddr2 := "cosmosvalcons1nx7n5uh0ztxsynn4sje6eyq2ud6rc6klc96w39" + consAddr2, _ := sdk.ConsAddressFromBech32(providerConsAddr2) + + providerKeeper.UpdateDenylist(ctx, consumerId, []string{providerConsAddr1, providerConsAddr2}) + + expectedDenylist := []providertypes.ProviderConsAddress{ + providertypes.NewProviderConsAddress(consAddr1), + providertypes.NewProviderConsAddress(consAddr2)} + require.Equal(t, expectedDenylist, providerKeeper.GetDenyList(ctx, consumerId)) +} + // Tests setting, getting and deleting parameters that are stored per-consumer chain. // The tests cover the following parameters: // - MinimumPowerInTopN @@ -535,3 +576,64 @@ func TestConsumerClientId(t *testing.T) { _, found = providerKeeper.GetClientIdToConsumerId(ctx, clientIds[1]) require.False(t, found) } + +func TestUpdateMinimumPowerInTopN(t *testing.T) { + providerKeeper, ctx, ctrl, mocks := testkeeper.GetProviderKeeperAndCtx(t, testkeeper.NewInMemKeeperParams(t)) + defer ctrl.Finish() + + consumerId := "0" + + // test case where Top N is 0 in which case there's no minimum power in top N + providerKeeper.SetConsumerPowerShapingParameters(ctx, consumerId, providertypes.PowerShapingParameters{ + Top_N: 0, + }) + + err := providerKeeper.UpdateMinimumPowerInTopN(ctx, consumerId, 0, 0) + require.NoError(t, err) + _, found := providerKeeper.GetMinimumPowerInTopN(ctx, consumerId) + require.False(t, found) + + // test cases where Top N > 0 and for this we mock some validators + powers := []int64{10, 20, 30} + validators := []stakingtypes.Validator{ + createStakingValidator(ctx, mocks, powers[0], 1), // this validator has ~16 of the total voting power + createStakingValidator(ctx, mocks, powers[1], 2), // this validator has ~33% of the total voting gpower + createStakingValidator(ctx, mocks, powers[2], 3), // this validator has 50% of the total voting power + } + mocks.MockStakingKeeper.EXPECT().GetBondedValidatorsByPower(gomock.Any()).Return(validators, nil).AnyTimes() + + maxProviderConsensusValidators := int64(3) + params := providerKeeper.GetParams(ctx) + params.MaxProviderConsensusValidators = maxProviderConsensusValidators + providerKeeper.SetParams(ctx, params) + + // when top N is 50, the minimum power is 30 (because top validator has to validate) + providerKeeper.SetConsumerPowerShapingParameters(ctx, consumerId, providertypes.PowerShapingParameters{ + Top_N: 50, + }) + err = providerKeeper.UpdateMinimumPowerInTopN(ctx, consumerId, 0, 50) + require.NoError(t, err) + minimumPowerInTopN, found := providerKeeper.GetMinimumPowerInTopN(ctx, consumerId) + require.True(t, found) + require.Equal(t, int64(30), minimumPowerInTopN) + + // when top N is 51, the minimum power is 20 (because top 2 validators have to validate) + providerKeeper.SetConsumerPowerShapingParameters(ctx, consumerId, providertypes.PowerShapingParameters{ + Top_N: 51, + }) + err = providerKeeper.UpdateMinimumPowerInTopN(ctx, consumerId, 50, 51) + require.NoError(t, err) + minimumPowerInTopN, found = providerKeeper.GetMinimumPowerInTopN(ctx, consumerId) + require.True(t, found) + require.Equal(t, int64(20), minimumPowerInTopN) + + // when top N is 100, the minimum power is 10 (that of the validator with the lowest power) + providerKeeper.SetConsumerPowerShapingParameters(ctx, consumerId, providertypes.PowerShapingParameters{ + Top_N: 100, + }) + err = providerKeeper.UpdateMinimumPowerInTopN(ctx, consumerId, 51, 100) + require.NoError(t, err) + minimumPowerInTopN, found = providerKeeper.GetMinimumPowerInTopN(ctx, consumerId) + require.True(t, found) + require.Equal(t, int64(10), minimumPowerInTopN) +} diff --git a/x/ccv/provider/keeper/permissionless.go b/x/ccv/provider/keeper/permissionless.go index caa89f75c7..99ba7910f9 100644 --- a/x/ccv/provider/keeper/permissionless.go +++ b/x/ccv/provider/keeper/permissionless.go @@ -292,57 +292,6 @@ func (k Keeper) RemoveOptedInConsumerId(ctx sdk.Context, providerAddr types.Prov return nil } -// UpdateAllowlist populates the allowlist store for the consumer chain with this consumer id -func (k Keeper) UpdateAllowlist(ctx sdk.Context, consumerId string, allowlist []string) { - k.DeleteAllowlist(ctx, consumerId) - for _, address := range allowlist { - consAddr, err := sdk.ConsAddressFromBech32(address) - if err != nil { - continue - } - - k.SetAllowlist(ctx, consumerId, types.NewProviderConsAddress(consAddr)) - } -} - -// UpdateDenylist populates the denylist store for the consumer chain with this consumer id -func (k Keeper) UpdateDenylist(ctx sdk.Context, consumerId string, denylist []string) { - k.DeleteDenylist(ctx, consumerId) - for _, address := range denylist { - consAddr, err := sdk.ConsAddressFromBech32(address) - if err != nil { - continue - } - - k.SetDenylist(ctx, consumerId, types.NewProviderConsAddress(consAddr)) - } - -} - -// UpdateMinimumPowerInTopN populates the minimum power in Top N for the consumer chain with this consumer id -func (k Keeper) UpdateMinimumPowerInTopN(ctx sdk.Context, consumerId string, oldTopN uint32, newTopN uint32) error { - // if the top N changes, we need to update the new minimum power in top N - if newTopN != oldTopN { - if newTopN > 0 { - // if the chain receives a non-zero top N value, store the minimum power in the top N - bondedValidators, err := k.GetLastProviderConsensusActiveValidators(ctx) - if err != nil { - return err - } - minPower, err := k.ComputeMinPowerInTopN(ctx, bondedValidators, newTopN) - if err != nil { - return err - } - k.SetMinimumPowerInTopN(ctx, consumerId, minPower) - } else { - // if the chain receives a zero top N value, we delete the min power - k.DeleteMinimumPowerInTopN(ctx, consumerId) - } - } - - return nil -} - // IsValidatorOptedInToChainId checks if the validator with `providerAddr` is opted into the chain with the specified `chainId`. // It returns `found == true` and the corresponding chain's `consumerId` if the validator is opted in. Otherwise, it returns an empty string // for `consumerId` and `found == false`. diff --git a/x/ccv/provider/keeper/permissionless_test.go b/x/ccv/provider/keeper/permissionless_test.go index 2b0534b8a5..e10326b844 100644 --- a/x/ccv/provider/keeper/permissionless_test.go +++ b/x/ccv/provider/keeper/permissionless_test.go @@ -6,12 +6,10 @@ import ( sdk "github.com/cosmos/cosmos-sdk/types" govv1 "github.com/cosmos/cosmos-sdk/x/gov/types/v1" - stakingtypes "github.com/cosmos/cosmos-sdk/x/staking/types" "github.com/cosmos/ibc-go/v8/modules/core/02-client/types" testkeeper "github.com/cosmos/interchain-security/v5/testutil/keeper" "github.com/cosmos/interchain-security/v5/x/ccv/provider/keeper" providertypes "github.com/cosmos/interchain-security/v5/x/ccv/provider/types" - "github.com/golang/mock/gomock" "github.com/stretchr/testify/require" ) @@ -298,105 +296,6 @@ func TestGetOptedInConsumerIds(t *testing.T) { require.Empty(t, consumers) } -func TestUpdateAllowlist(t *testing.T) { - providerKeeper, ctx, ctrl, _ := testkeeper.GetProviderKeeperAndCtx(t, testkeeper.NewInMemKeeperParams(t)) - defer ctrl.Finish() - - consumerId := "0" - - providerConsAddr1 := "cosmosvalcons1qmq08eruchr5sf5s3rwz7djpr5a25f7xw4mceq" - consAddr1, _ := sdk.ConsAddressFromBech32(providerConsAddr1) - providerConsAddr2 := "cosmosvalcons1nx7n5uh0ztxsynn4sje6eyq2ud6rc6klc96w39" - consAddr2, _ := sdk.ConsAddressFromBech32(providerConsAddr2) - - providerKeeper.UpdateAllowlist(ctx, consumerId, []string{providerConsAddr1, providerConsAddr2}) - - expectedAllowlist := []providertypes.ProviderConsAddress{ - providertypes.NewProviderConsAddress(consAddr1), - providertypes.NewProviderConsAddress(consAddr2)} - require.Equal(t, expectedAllowlist, providerKeeper.GetAllowList(ctx, consumerId)) -} - -func TestUpdateDenylist(t *testing.T) { - providerKeeper, ctx, ctrl, _ := testkeeper.GetProviderKeeperAndCtx(t, testkeeper.NewInMemKeeperParams(t)) - defer ctrl.Finish() - - consumerId := "0" - - providerConsAddr1 := "cosmosvalcons1qmq08eruchr5sf5s3rwz7djpr5a25f7xw4mceq" - consAddr1, _ := sdk.ConsAddressFromBech32(providerConsAddr1) - providerConsAddr2 := "cosmosvalcons1nx7n5uh0ztxsynn4sje6eyq2ud6rc6klc96w39" - consAddr2, _ := sdk.ConsAddressFromBech32(providerConsAddr2) - - providerKeeper.UpdateDenylist(ctx, consumerId, []string{providerConsAddr1, providerConsAddr2}) - - expectedDenylist := []providertypes.ProviderConsAddress{ - providertypes.NewProviderConsAddress(consAddr1), - providertypes.NewProviderConsAddress(consAddr2)} - require.Equal(t, expectedDenylist, providerKeeper.GetDenyList(ctx, consumerId)) -} - -func TestUpdateMinimumPowerInTopN(t *testing.T) { - providerKeeper, ctx, ctrl, mocks := testkeeper.GetProviderKeeperAndCtx(t, testkeeper.NewInMemKeeperParams(t)) - defer ctrl.Finish() - - consumerId := "0" - - // test case where Top N is 0 in which case there's no minimum power in top N - providerKeeper.SetConsumerPowerShapingParameters(ctx, consumerId, providertypes.PowerShapingParameters{ - Top_N: 0, - }) - - err := providerKeeper.UpdateMinimumPowerInTopN(ctx, consumerId, 0, 0) - require.NoError(t, err) - _, found := providerKeeper.GetMinimumPowerInTopN(ctx, consumerId) - require.False(t, found) - - // test cases where Top N > 0 and for this we mock some validators - powers := []int64{10, 20, 30} - validators := []stakingtypes.Validator{ - createStakingValidator(ctx, mocks, powers[0], 1), // this validator has ~16 of the total voting power - createStakingValidator(ctx, mocks, powers[1], 2), // this validator has ~33% of the total voting gpower - createStakingValidator(ctx, mocks, powers[2], 3), // this validator has 50% of the total voting power - } - mocks.MockStakingKeeper.EXPECT().GetBondedValidatorsByPower(gomock.Any()).Return(validators, nil).AnyTimes() - - maxProviderConsensusValidators := int64(3) - params := providerKeeper.GetParams(ctx) - params.MaxProviderConsensusValidators = maxProviderConsensusValidators - providerKeeper.SetParams(ctx, params) - - // when top N is 50, the minimum power is 30 (because top validator has to validate) - providerKeeper.SetConsumerPowerShapingParameters(ctx, consumerId, providertypes.PowerShapingParameters{ - Top_N: 50, - }) - err = providerKeeper.UpdateMinimumPowerInTopN(ctx, consumerId, 0, 50) - require.NoError(t, err) - minimumPowerInTopN, found := providerKeeper.GetMinimumPowerInTopN(ctx, consumerId) - require.True(t, found) - require.Equal(t, int64(30), minimumPowerInTopN) - - // when top N is 51, the minimum power is 20 (because top 2 validators have to validate) - providerKeeper.SetConsumerPowerShapingParameters(ctx, consumerId, providertypes.PowerShapingParameters{ - Top_N: 51, - }) - err = providerKeeper.UpdateMinimumPowerInTopN(ctx, consumerId, 50, 51) - require.NoError(t, err) - minimumPowerInTopN, found = providerKeeper.GetMinimumPowerInTopN(ctx, consumerId) - require.True(t, found) - require.Equal(t, int64(20), minimumPowerInTopN) - - // when top N is 100, the minimum power is 10 (that of the validator with the lowest power) - providerKeeper.SetConsumerPowerShapingParameters(ctx, consumerId, providertypes.PowerShapingParameters{ - Top_N: 100, - }) - err = providerKeeper.UpdateMinimumPowerInTopN(ctx, consumerId, 51, 100) - require.NoError(t, err) - minimumPowerInTopN, found = providerKeeper.GetMinimumPowerInTopN(ctx, consumerId) - require.True(t, found) - require.Equal(t, int64(10), minimumPowerInTopN) -} - func TestIsValidatorOptedInToChain(t *testing.T) { providerKeeper, ctx, ctrl, _ := testkeeper.GetProviderKeeperAndCtx(t, testkeeper.NewInMemKeeperParams(t)) defer ctrl.Finish() From 2a6ab1475effb26021bd9681f9ba66ead5d475e5 Mon Sep 17 00:00:00 2001 From: mpoke Date: Thu, 29 Aug 2024 19:57:49 +0200 Subject: [PATCH 06/13] move methods around --- x/ccv/provider/keeper/consumer_lifecycle.go | 42 ++++++ .../keeper/consumer_lifecycle_test.go | 66 +++++++++ x/ccv/provider/keeper/keeper_test.go | 29 ++-- x/ccv/provider/keeper/permissionless.go | 95 ------------- x/ccv/provider/keeper/permissionless_test.go | 127 ------------------ 5 files changed, 122 insertions(+), 237 deletions(-) diff --git a/x/ccv/provider/keeper/consumer_lifecycle.go b/x/ccv/provider/keeper/consumer_lifecycle.go index 65cdca8b6a..319ac9d722 100644 --- a/x/ccv/provider/keeper/consumer_lifecycle.go +++ b/x/ccv/provider/keeper/consumer_lifecycle.go @@ -22,6 +22,48 @@ import ( ccv "github.com/cosmos/interchain-security/v5/x/ccv/types" ) +// PrepareConsumerForLaunch prepares to move the launch of a consumer chain from the previous spawn time to spawn time. +// Previous spawn time can correspond to its zero value if the validator was not previously set for launch. +func (k Keeper) PrepareConsumerForLaunch(ctx sdk.Context, consumerId string, previousSpawnTime time.Time, spawnTime time.Time) error { + if !previousSpawnTime.Equal(time.Time{}) { + // if this is not the first initialization and hence `previousSpawnTime` does not contain the zero value of `Time` + // remove the consumer id from the previous spawn time + err := k.RemoveConsumerToBeLaunched(ctx, consumerId, previousSpawnTime) + if err != nil { + return err + } + } + return k.AppendConsumerToBeLaunched(ctx, consumerId, spawnTime) +} + +// CanLaunch checks on whether the consumer with `consumerId` has set all the initialization parameters set and hence +// is ready to launch and at what spawn time +// TODO (PERMISSIONLESS): could remove, all fields should be there because we validate the initialization parameters +func (k Keeper) CanLaunch(ctx sdk.Context, consumerId string) (time.Time, bool) { + // a chain that is already launched or stopped cannot launch again + phase := k.GetConsumerPhase(ctx, consumerId) + if phase == types.ConsumerPhase_CONSUMER_PHASE_LAUNCHED || phase == types.ConsumerPhase_CONSUMER_PHASE_STOPPED { + return time.Time{}, false + } + + initializationParameters, err := k.GetConsumerInitializationParameters(ctx, consumerId) + if err != nil { + return time.Time{}, false + } + + spawnTimeIsNotZero := !initializationParameters.SpawnTime.Equal(time.Time{}) + + genesisHashSet := initializationParameters.GenesisHash != nil + binaryHashSet := initializationParameters.BinaryHash != nil + + consumerRedistributionFractionSet := initializationParameters.ConsumerRedistributionFraction != "" + blocksPerDistributionTransmissionSet := initializationParameters.BlocksPerDistributionTransmission > 0 + historicalEntriesSet := initializationParameters.HistoricalEntries > 0 + + return initializationParameters.SpawnTime, spawnTimeIsNotZero && genesisHashSet && binaryHashSet && consumerRedistributionFractionSet && + blocksPerDistributionTransmissionSet && historicalEntriesSet +} + // BeginBlockLaunchConsumers launches initialized consumers that are ready to launch func (k Keeper) BeginBlockLaunchConsumers(ctx sdk.Context) { // TODO (PERMISSIONLESS): we can parameterize the limit diff --git a/x/ccv/provider/keeper/consumer_lifecycle_test.go b/x/ccv/provider/keeper/consumer_lifecycle_test.go index e05bdb460c..b41791651b 100644 --- a/x/ccv/provider/keeper/consumer_lifecycle_test.go +++ b/x/ccv/provider/keeper/consumer_lifecycle_test.go @@ -27,6 +27,72 @@ import ( ccvtypes "github.com/cosmos/interchain-security/v5/x/ccv/types" ) +func TestPrepareConsumerForLaunch(t *testing.T) { + providerKeeper, ctx, ctrl, _ := testkeeper.GetProviderKeeperAndCtx(t, testkeeper.NewInMemKeeperParams(t)) + defer ctrl.Finish() + + spawnTime := time.Now().UTC() + err := providerKeeper.PrepareConsumerForLaunch(ctx, "consumerId", time.Time{}, spawnTime) + require.NoError(t, err) + + consumers, err := providerKeeper.GetConsumersToBeLaunched(ctx, spawnTime) + require.NoError(t, err) + require.Equal(t, providertypes.ConsumerIds{Ids: []string{"consumerId"}}, consumers) + + nextSpawnTime := spawnTime.Add(time.Hour) + err = providerKeeper.PrepareConsumerForLaunch(ctx, "consumerId", spawnTime, nextSpawnTime) + require.NoError(t, err) + + consumers, err = providerKeeper.GetConsumersToBeLaunched(ctx, spawnTime) + require.NoError(t, err) + require.Empty(t, consumers) + + consumers, err = providerKeeper.GetConsumersToBeLaunched(ctx, nextSpawnTime) + require.NoError(t, err) + require.Equal(t, providertypes.ConsumerIds{Ids: []string{"consumerId"}}, consumers) +} + +func TestCanLaunch(t *testing.T) { + providerKeeper, ctx, ctrl, _ := testkeeper.GetProviderKeeperAndCtx(t, testkeeper.NewInMemKeeperParams(t)) + defer ctrl.Finish() + + // cannot launch an unknown chain + _, canLaunch := providerKeeper.CanLaunch(ctx, "consumerId") + require.False(t, canLaunch) + + // cannot launch a chain without initialization parameters + providerKeeper.SetConsumerPhase(ctx, "consumerId", providertypes.ConsumerPhase_CONSUMER_PHASE_INITIALIZED) + _, canLaunch = providerKeeper.CanLaunch(ctx, "consumerId") + require.False(t, canLaunch) + + // set valid initialization parameters + initializationParameters := testkeeper.GetTestInitializationParameters() + err := providerKeeper.SetConsumerInitializationParameters(ctx, "consumerId", initializationParameters) + require.NoError(t, err) + + // cannot launch a launched chain + providerKeeper.SetConsumerPhase(ctx, "consumerId", providertypes.ConsumerPhase_CONSUMER_PHASE_LAUNCHED) + _, canLaunch = providerKeeper.CanLaunch(ctx, "consumerId") + require.False(t, canLaunch) + + // cannot launch a stopped chain + providerKeeper.SetConsumerPhase(ctx, "consumerId", providertypes.ConsumerPhase_CONSUMER_PHASE_STOPPED) + _, canLaunch = providerKeeper.CanLaunch(ctx, "consumerId") + require.False(t, canLaunch) + + // initialized chain can launch + providerKeeper.SetConsumerPhase(ctx, "consumerId", providertypes.ConsumerPhase_CONSUMER_PHASE_INITIALIZED) + _, canLaunch = providerKeeper.CanLaunch(ctx, "consumerId") + require.True(t, canLaunch) + + // chain cannot launch without a genesis hash + initializationParameters.GenesisHash = nil + err = providerKeeper.SetConsumerInitializationParameters(ctx, "consumerId", initializationParameters) + _, canLaunch = providerKeeper.CanLaunch(ctx, "consumerId") + require.NoError(t, err) + require.False(t, canLaunch) +} + // TestBeginBlockInit directly tests BeginBlockLaunchConsumers against the spec using helpers defined above. func TestBeginBlockLaunchConsumers(t *testing.T) { now := time.Now().UTC() diff --git a/x/ccv/provider/keeper/keeper_test.go b/x/ccv/provider/keeper/keeper_test.go index a8cb2583bf..5431c95390 100644 --- a/x/ccv/provider/keeper/keeper_test.go +++ b/x/ccv/provider/keeper/keeper_test.go @@ -20,7 +20,6 @@ import ( cryptotestutil "github.com/cosmos/interchain-security/v5/testutil/crypto" testkeeper "github.com/cosmos/interchain-security/v5/testutil/keeper" - "github.com/cosmos/interchain-security/v5/x/ccv/provider/types" providertypes "github.com/cosmos/interchain-security/v5/x/ccv/provider/types" ccv "github.com/cosmos/interchain-security/v5/x/ccv/types" ) @@ -58,7 +57,7 @@ func TestGetAllValsetUpdateBlockHeights(t *testing.T) { pk, ctx, ctrl, _ := testkeeper.GetProviderKeeperAndCtx(t, testkeeper.NewInMemKeeperParams(t)) defer ctrl.Finish() - cases := []types.ValsetUpdateIdToHeight{ + cases := []providertypes.ValsetUpdateIdToHeight{ { ValsetUpdateId: 2, Height: 22, @@ -291,10 +290,10 @@ func TestGetAllOptedIn(t *testing.T) { providerKeeper, ctx, ctrl, _ := testkeeper.GetProviderKeeperAndCtx(t, testkeeper.NewInMemKeeperParams(t)) defer ctrl.Finish() - expectedOptedInValidators := []types.ProviderConsAddress{ - types.NewProviderConsAddress([]byte("providerAddr1")), - types.NewProviderConsAddress([]byte("providerAddr2")), - types.NewProviderConsAddress([]byte("providerAddr3")), + expectedOptedInValidators := []providertypes.ProviderConsAddress{ + providertypes.NewProviderConsAddress([]byte("providerAddr1")), + providertypes.NewProviderConsAddress([]byte("providerAddr2")), + providertypes.NewProviderConsAddress([]byte("providerAddr3")), } for _, expectedOptedInValidator := range expectedOptedInValidators { @@ -304,7 +303,7 @@ func TestGetAllOptedIn(t *testing.T) { actualOptedInValidators := providerKeeper.GetAllOptedIn(ctx, "consumerId") // sort validators first to be able to compare - sortOptedInValidators := func(addresses []types.ProviderConsAddress) { + sortOptedInValidators := func(addresses []providertypes.ProviderConsAddress) { sort.Slice(addresses, func(i, j int) bool { return bytes.Compare(addresses[i].ToSdkConsAddr(), addresses[j].ToSdkConsAddr()) < 0 }) @@ -319,8 +318,8 @@ func TestOptedIn(t *testing.T) { providerKeeper, ctx, ctrl, _ := testkeeper.GetProviderKeeperAndCtx(t, testkeeper.NewInMemKeeperParams(t)) defer ctrl.Finish() - optedInValidator1 := types.NewProviderConsAddress([]byte("providerAddr1")) - optedInValidator2 := types.NewProviderConsAddress([]byte("providerAddr2")) + optedInValidator1 := providertypes.NewProviderConsAddress([]byte("providerAddr1")) + optedInValidator2 := providertypes.NewProviderConsAddress([]byte("providerAddr2")) require.False(t, providerKeeper.IsOptedIn(ctx, "consumerId", optedInValidator1)) providerKeeper.SetOptedIn(ctx, "consumerId", optedInValidator1) @@ -342,8 +341,8 @@ func TestConsumerCommissionRate(t *testing.T) { providerKeeper, ctx, ctrl, _ := testkeeper.GetProviderKeeperAndCtx(t, testkeeper.NewInMemKeeperParams(t)) defer ctrl.Finish() - providerAddr1 := types.NewProviderConsAddress([]byte("providerAddr1")) - providerAddr2 := types.NewProviderConsAddress([]byte("providerAddr2")) + providerAddr1 := providertypes.NewProviderConsAddress([]byte("providerAddr1")) + providerAddr2 := providertypes.NewProviderConsAddress([]byte("providerAddr2")) cr, found := providerKeeper.GetConsumerCommissionRate(ctx, "consumerId", providerAddr1) require.False(t, found) @@ -383,14 +382,14 @@ func TestAllowlist(t *testing.T) { // no validator was allowlisted and hence the allowlist is empty require.True(t, providerKeeper.IsAllowlistEmpty(ctx, chainID)) - providerAddr1 := types.NewProviderConsAddress([]byte("providerAddr1")) + providerAddr1 := providertypes.NewProviderConsAddress([]byte("providerAddr1")) providerKeeper.SetAllowlist(ctx, chainID, providerAddr1) require.True(t, providerKeeper.IsAllowlisted(ctx, chainID, providerAddr1)) // allowlist is not empty anymore require.False(t, providerKeeper.IsAllowlistEmpty(ctx, chainID)) - providerAddr2 := types.NewProviderConsAddress([]byte("providerAddr2")) + providerAddr2 := providertypes.NewProviderConsAddress([]byte("providerAddr2")) providerKeeper.SetAllowlist(ctx, chainID, providerAddr2) require.True(t, providerKeeper.IsAllowlisted(ctx, chainID, providerAddr2)) require.False(t, providerKeeper.IsAllowlistEmpty(ctx, chainID)) @@ -430,14 +429,14 @@ func TestDenylist(t *testing.T) { // no validator was denylisted and hence the denylist is empty require.True(t, providerKeeper.IsDenylistEmpty(ctx, chainID)) - providerAddr1 := types.NewProviderConsAddress([]byte("providerAddr1")) + providerAddr1 := providertypes.NewProviderConsAddress([]byte("providerAddr1")) providerKeeper.SetDenylist(ctx, chainID, providerAddr1) require.True(t, providerKeeper.IsDenylisted(ctx, chainID, providerAddr1)) // denylist is not empty anymore require.False(t, providerKeeper.IsDenylistEmpty(ctx, chainID)) - providerAddr2 := types.NewProviderConsAddress([]byte("providerAddr2")) + providerAddr2 := providertypes.NewProviderConsAddress([]byte("providerAddr2")) providerKeeper.SetDenylist(ctx, chainID, providerAddr2) require.True(t, providerKeeper.IsDenylisted(ctx, chainID, providerAddr2)) require.False(t, providerKeeper.IsDenylistEmpty(ctx, chainID)) diff --git a/x/ccv/provider/keeper/permissionless.go b/x/ccv/provider/keeper/permissionless.go index 99ba7910f9..695db64463 100644 --- a/x/ccv/provider/keeper/permissionless.go +++ b/x/ccv/provider/keeper/permissionless.go @@ -4,10 +4,8 @@ import ( "encoding/binary" "fmt" "strconv" - "time" sdk "github.com/cosmos/cosmos-sdk/types" - govv1 "github.com/cosmos/cosmos-sdk/x/gov/types/v1" "github.com/cosmos/interchain-security/v5/x/ccv/provider/types" ) @@ -320,96 +318,3 @@ func (k Keeper) IsValidatorOptedInToChainId(ctx sdk.Context, providerAddr types. } return "", false } - -// PrepareConsumerForLaunch prepares to move the launch of a consumer chain from the previous spawn time to spawn time. -// Previous spawn time can correspond to its zero value if the validator was not previously set for launch. -func (k Keeper) PrepareConsumerForLaunch(ctx sdk.Context, consumerId string, previousSpawnTime time.Time, spawnTime time.Time) error { - if !previousSpawnTime.Equal(time.Time{}) { - // if this is not the first initialization and hence `previousSpawnTime` does not contain the zero value of `Time` - // remove the consumer id from the previous spawn time - err := k.RemoveConsumerToBeLaunched(ctx, consumerId, previousSpawnTime) - if err != nil { - return err - } - } - return k.AppendConsumerToBeLaunched(ctx, consumerId, spawnTime) -} - -// CanLaunch checks on whether the consumer with `consumerId` has set all the initialization parameters set and hence -// is ready to launch and at what spawn time -// TODO (PERMISSIONLESS): could remove, all fields should be there because we validate the initialization parameters -func (k Keeper) CanLaunch(ctx sdk.Context, consumerId string) (time.Time, bool) { - // a chain that is already launched or stopped cannot launch again - phase := k.GetConsumerPhase(ctx, consumerId) - if phase == types.ConsumerPhase_CONSUMER_PHASE_LAUNCHED || phase == types.ConsumerPhase_CONSUMER_PHASE_STOPPED { - return time.Time{}, false - } - - initializationParameters, err := k.GetConsumerInitializationParameters(ctx, consumerId) - if err != nil { - return time.Time{}, false - } - - spawnTimeIsNotZero := !initializationParameters.SpawnTime.Equal(time.Time{}) - - genesisHashSet := initializationParameters.GenesisHash != nil - binaryHashSet := initializationParameters.BinaryHash != nil - - consumerRedistributionFractionSet := initializationParameters.ConsumerRedistributionFraction != "" - blocksPerDistributionTransmissionSet := initializationParameters.BlocksPerDistributionTransmission > 0 - historicalEntriesSet := initializationParameters.HistoricalEntries > 0 - - return initializationParameters.SpawnTime, spawnTimeIsNotZero && genesisHashSet && binaryHashSet && consumerRedistributionFractionSet && - blocksPerDistributionTransmissionSet && historicalEntriesSet -} - -// HasAtMostOnceCorrectMsgUpdateConsumer checks that the proposal has at most one `MsgUpdateConsumer` that is -// correctly set (i.e., the owner address of the to-be-updated consumer corresponds to the gov module). Returns -// the single `MsgUpdateConsumer` message if only one correctly set exists. -func (k Keeper) HasAtMostOnceCorrectMsgUpdateConsumer(ctx sdk.Context, proposal *govv1.Proposal) (*types.MsgUpdateConsumer, error) { - var msgUpdateConsumer *types.MsgUpdateConsumer - for _, msg := range proposal.GetMessages() { - sdkMsg, isMsgUpdateConsumer := msg.GetCachedValue().(*types.MsgUpdateConsumer) - if isMsgUpdateConsumer { - // A `MsgUpdateConsumer` can only succeed if the owner of the consumer chain is the gov module. - // If that's not the case, we immediately fail the proposal. - // Note that someone could potentially change the owner of a chain to be that of the gov module - // while a proposal is active and before the proposal is executed. Even then, we still do not allow - // `MsgUpdateConsumer` proposals if the owner of the chain is not the gov module to avoid someone forgetting - // to change the owner address while the proposal is active. - ownerAddress, err := k.GetConsumerOwnerAddress(ctx, sdkMsg.ConsumerId) - if err != nil { - return nil, fmt.Errorf("cannot find owner address for consumer with consumer id (%s): %s", sdkMsg.ConsumerId, err.Error()) - } else if ownerAddress != k.GetAuthority() { - return nil, fmt.Errorf("owner address (%s) is not the gov module (%s)", ownerAddress, k.GetAuthority()) - } - - if msgUpdateConsumer != nil { - return nil, fmt.Errorf("proposal can contain at most one `MsgUpdateConsumer` message") - } - msgUpdateConsumer = sdkMsg - } - } - return msgUpdateConsumer, nil -} - -// DoesNotHaveDeprecatedMessage checks that the provided proposal does not contain any deprecated messages and returns -// an error if this is the case -func DoesNotHaveDeprecatedMessage(proposal *govv1.Proposal) error { - for _, msg := range proposal.GetMessages() { - // if the proposal contains a deprecated message, cancel the proposal - _, isMsgConsumerAddition := msg.GetCachedValue().(*types.MsgConsumerAddition) - if isMsgConsumerAddition { - return fmt.Errorf("proposal cannot contain deprecated `MsgConsumerAddition`; use `MsgCreateConsumer` instead") - } - _, isMsgConsumerModification := msg.GetCachedValue().(*types.MsgConsumerModification) - if isMsgConsumerModification { - return fmt.Errorf("proposal cannot contain deprecated `MsgConsumerModification`; use `MsgUpdateConsumer` instead") - } - _, isMsgConsumerRemoval := msg.GetCachedValue().(*types.MsgConsumerRemoval) - if isMsgConsumerRemoval { - return fmt.Errorf("proposal cannot contain deprecated `MsgConsumerRemoval`; use `MsgRemoveConsumer` instead") - } - } - return nil -} diff --git a/x/ccv/provider/keeper/permissionless_test.go b/x/ccv/provider/keeper/permissionless_test.go index e10326b844..20a929f595 100644 --- a/x/ccv/provider/keeper/permissionless_test.go +++ b/x/ccv/provider/keeper/permissionless_test.go @@ -4,11 +4,8 @@ import ( "testing" "time" - sdk "github.com/cosmos/cosmos-sdk/types" - govv1 "github.com/cosmos/cosmos-sdk/x/gov/types/v1" "github.com/cosmos/ibc-go/v8/modules/core/02-client/types" testkeeper "github.com/cosmos/interchain-security/v5/testutil/keeper" - "github.com/cosmos/interchain-security/v5/x/ccv/provider/keeper" providertypes "github.com/cosmos/interchain-security/v5/x/ccv/provider/types" "github.com/stretchr/testify/require" ) @@ -313,127 +310,3 @@ func TestIsValidatorOptedInToChain(t *testing.T) { require.True(t, found) require.Equal(t, expectedConsumerId, actualConsumerId) } - -func TestPrepareConsumerForLaunch(t *testing.T) { - providerKeeper, ctx, ctrl, _ := testkeeper.GetProviderKeeperAndCtx(t, testkeeper.NewInMemKeeperParams(t)) - defer ctrl.Finish() - - spawnTime := time.Now().UTC() - err := providerKeeper.PrepareConsumerForLaunch(ctx, "consumerId", time.Time{}, spawnTime) - require.NoError(t, err) - - consumers, err := providerKeeper.GetConsumersToBeLaunched(ctx, spawnTime) - require.NoError(t, err) - require.Equal(t, providertypes.ConsumerIds{Ids: []string{"consumerId"}}, consumers) - - nextSpawnTime := spawnTime.Add(time.Hour) - err = providerKeeper.PrepareConsumerForLaunch(ctx, "consumerId", spawnTime, nextSpawnTime) - require.NoError(t, err) - - consumers, err = providerKeeper.GetConsumersToBeLaunched(ctx, spawnTime) - require.NoError(t, err) - require.Empty(t, consumers) - - consumers, err = providerKeeper.GetConsumersToBeLaunched(ctx, nextSpawnTime) - require.NoError(t, err) - require.Equal(t, providertypes.ConsumerIds{Ids: []string{"consumerId"}}, consumers) -} - -func TestCanLaunch(t *testing.T) { - providerKeeper, ctx, ctrl, _ := testkeeper.GetProviderKeeperAndCtx(t, testkeeper.NewInMemKeeperParams(t)) - defer ctrl.Finish() - - // cannot launch an unknown chain - _, canLaunch := providerKeeper.CanLaunch(ctx, "consumerId") - require.False(t, canLaunch) - - // cannot launch a chain without initialization parameters - providerKeeper.SetConsumerPhase(ctx, "consumerId", providertypes.ConsumerPhase_CONSUMER_PHASE_INITIALIZED) - _, canLaunch = providerKeeper.CanLaunch(ctx, "consumerId") - require.False(t, canLaunch) - - // set valid initialization parameters - initializationParameters := testkeeper.GetTestInitializationParameters() - err := providerKeeper.SetConsumerInitializationParameters(ctx, "consumerId", initializationParameters) - require.NoError(t, err) - - // cannot launch a launched chain - providerKeeper.SetConsumerPhase(ctx, "consumerId", providertypes.ConsumerPhase_CONSUMER_PHASE_LAUNCHED) - _, canLaunch = providerKeeper.CanLaunch(ctx, "consumerId") - require.False(t, canLaunch) - - // cannot launch a stopped chain - providerKeeper.SetConsumerPhase(ctx, "consumerId", providertypes.ConsumerPhase_CONSUMER_PHASE_STOPPED) - _, canLaunch = providerKeeper.CanLaunch(ctx, "consumerId") - require.False(t, canLaunch) - - // initialized chain can launch - providerKeeper.SetConsumerPhase(ctx, "consumerId", providertypes.ConsumerPhase_CONSUMER_PHASE_INITIALIZED) - _, canLaunch = providerKeeper.CanLaunch(ctx, "consumerId") - require.True(t, canLaunch) - - // chain cannot launch without a genesis hash - initializationParameters.GenesisHash = nil - err = providerKeeper.SetConsumerInitializationParameters(ctx, "consumerId", initializationParameters) - _, canLaunch = providerKeeper.CanLaunch(ctx, "consumerId") - require.NoError(t, err) - require.False(t, canLaunch) -} - -func TestHasAtMostOnceCorrectMsgUpdateConsumer(t *testing.T) { - providerKeeper, ctx, ctrl, _ := testkeeper.GetProviderKeeperAndCtx(t, testkeeper.NewInMemKeeperParams(t)) - defer ctrl.Finish() - - expectedMsgUpdateConsumer := providertypes.MsgUpdateConsumer{Signer: "signer", ConsumerId: "consumerId", NewOwnerAddress: "new owner address"} - - proposal, err := govv1.NewProposal([]sdk.Msg{&expectedMsgUpdateConsumer}, 1, time.Now(), time.Now().Add(1*time.Hour), "metadata", "title", "summary", sdk.AccAddress{}, false) - require.NoError(t, err) - - _, err = providerKeeper.HasAtMostOnceCorrectMsgUpdateConsumer(ctx, &proposal) - require.ErrorContains(t, err, "cannot find owner address") - - // set owner address that is not the gov module - providerKeeper.SetConsumerOwnerAddress(ctx, "consumerId", "owner address") - _, err = providerKeeper.HasAtMostOnceCorrectMsgUpdateConsumer(ctx, &proposal) - require.ErrorContains(t, err, "is not the gov module") - - // set owner address that is the gov module - providerKeeper.SetConsumerOwnerAddress(ctx, "consumerId", "cosmos10d07y265gmmuvt4z0w9aw880jnsr700j6zn9kn") - actualMsgUpdateConsumer, err := providerKeeper.HasAtMostOnceCorrectMsgUpdateConsumer(ctx, &proposal) - require.NoError(t, err) - require.Equal(t, expectedMsgUpdateConsumer, *actualMsgUpdateConsumer) - - // a proposal with 2 `MsgUpdateConsumer` messages - invalidProposal, err := govv1.NewProposal([]sdk.Msg{&expectedMsgUpdateConsumer, &expectedMsgUpdateConsumer}, 1, time.Now(), time.Now().Add(1*time.Hour), "metadata", "title", "summary", sdk.AccAddress{}, false) - require.NoError(t, err) - actualMsgUpdateConsumer, err = providerKeeper.HasAtMostOnceCorrectMsgUpdateConsumer(ctx, &invalidProposal) - require.ErrorContains(t, err, "proposal can contain at most one") - require.Nil(t, actualMsgUpdateConsumer) -} - -func TestDoesNotHaveDeprecatedMessage(t *testing.T) { - msgConsumerAddition := providertypes.MsgConsumerAddition{} - proposal, err := govv1.NewProposal([]sdk.Msg{&msgConsumerAddition}, 1, time.Now(), time.Now().Add(1*time.Hour), "metadata", "title", "summary", sdk.AccAddress{}, false) - require.NoError(t, err) - err = keeper.DoesNotHaveDeprecatedMessage(&proposal) - require.ErrorContains(t, err, "cannot contain deprecated `MsgConsumerAddition`") - - msgConsumerModification := providertypes.MsgConsumerModification{} - proposal, err = govv1.NewProposal([]sdk.Msg{&msgConsumerModification}, 1, time.Now(), time.Now().Add(1*time.Hour), "metadata", "title", "summary", sdk.AccAddress{}, false) - require.NoError(t, err) - err = keeper.DoesNotHaveDeprecatedMessage(&proposal) - require.ErrorContains(t, err, "cannot contain deprecated `MsgConsumerModification`") - - msgConsumerRemoval := providertypes.MsgConsumerRemoval{} - proposal, err = govv1.NewProposal([]sdk.Msg{&msgConsumerRemoval}, 1, time.Now(), time.Now().Add(1*time.Hour), "metadata", "title", "summary", sdk.AccAddress{}, false) - require.NoError(t, err) - err = keeper.DoesNotHaveDeprecatedMessage(&proposal) - require.ErrorContains(t, err, "cannot contain deprecated `MsgConsumerRemoval`") - - // a proposal with no deprecated messages - msgUpdateConsumer := providertypes.MsgUpdateConsumer{Signer: "signer", ConsumerId: "consumerId", NewOwnerAddress: "new owner address"} - proposal, err = govv1.NewProposal([]sdk.Msg{&msgUpdateConsumer}, 1, time.Now(), time.Now().Add(1*time.Hour), "metadata", "title", "summary", sdk.AccAddress{}, false) - require.NoError(t, err) - err = keeper.DoesNotHaveDeprecatedMessage(&proposal) - require.Nil(t, err) -} From 452fe04bfc724539f34ebcb9e9b3e99b467a1487 Mon Sep 17 00:00:00 2001 From: mpoke Date: Thu, 29 Aug 2024 21:17:01 +0200 Subject: [PATCH 07/13] handle ChangeRewardDenoms --- x/ccv/provider/keeper/distribution.go | 26 ++++++++++++++++++ x/ccv/provider/keeper/legacy_proposal.go | 35 ------------------------ x/ccv/provider/keeper/msg_server.go | 18 +++++++++--- x/ccv/provider/keeper/proposal.go | 18 ------------ x/ccv/provider/proposal_handler.go | 2 +- x/ccv/provider/types/errors.go | 1 + x/ccv/provider/types/events.go | 6 ++-- 7 files changed, 45 insertions(+), 61 deletions(-) delete mode 100644 x/ccv/provider/keeper/legacy_proposal.go delete mode 100644 x/ccv/provider/keeper/proposal.go diff --git a/x/ccv/provider/keeper/distribution.go b/x/ccv/provider/keeper/distribution.go index 9d67181158..69964708df 100644 --- a/x/ccv/provider/keeper/distribution.go +++ b/x/ccv/provider/keeper/distribution.go @@ -361,3 +361,29 @@ func (k Keeper) HandleSetConsumerCommissionRate(ctx sdk.Context, consumerId stri commissionRate, ) } + +// TODO: this method needs to be tested +func (k Keeper) ChangeRewardDenoms(ctx sdk.Context, denomsToAdd, denomsToRemove []string) []sdk.Attribute { + eventAttributes := []sdk.Attribute{} + for _, denomToAdd := range denomsToAdd { + // Log error and move on if one of the denoms is already registered + if k.ConsumerRewardDenomExists(ctx, denomToAdd) { + ctx.Logger().Error("denom %s already registered", denomToAdd) + continue + } + k.SetConsumerRewardDenom(ctx, denomToAdd) + + eventAttributes = append(eventAttributes, sdk.NewAttribute(types.AttributeAddConsumerRewardDenom, denomToAdd)) + } + for _, denomToRemove := range denomsToRemove { + // Log error and move on if one of the denoms is not registered + if !k.ConsumerRewardDenomExists(ctx, denomToRemove) { + ctx.Logger().Error("denom %s not registered", denomToRemove) + continue + } + k.DeleteConsumerRewardDenom(ctx, denomToRemove) + + eventAttributes = append(eventAttributes, sdk.NewAttribute(types.AttributeRemoveConsumerRewardDenom, denomToRemove)) + } + return eventAttributes +} diff --git a/x/ccv/provider/keeper/legacy_proposal.go b/x/ccv/provider/keeper/legacy_proposal.go deleted file mode 100644 index a3e6ed2f30..0000000000 --- a/x/ccv/provider/keeper/legacy_proposal.go +++ /dev/null @@ -1,35 +0,0 @@ -package keeper - -import ( - sdk "github.com/cosmos/cosmos-sdk/types" - - "github.com/cosmos/interchain-security/v5/x/ccv/provider/types" -) - -func (k Keeper) HandleLegacyConsumerRewardDenomProposal(ctx sdk.Context, p *types.ChangeRewardDenomsProposal) error { - for _, denomToAdd := range p.DenomsToAdd { - // Log error and move on if one of the denoms is already registered - if k.ConsumerRewardDenomExists(ctx, denomToAdd) { - ctx.Logger().Error("denom %s already registered", denomToAdd) - continue - } - k.SetConsumerRewardDenom(ctx, denomToAdd) - ctx.EventManager().EmitEvent(sdk.NewEvent( - types.EventTypeAddConsumerRewardDenom, - sdk.NewAttribute(types.AttributeConsumerRewardDenom, denomToAdd), - )) - } - for _, denomToRemove := range p.DenomsToRemove { - // Log error and move on if one of the denoms is not registered - if !k.ConsumerRewardDenomExists(ctx, denomToRemove) { - ctx.Logger().Error("denom %s not registered", denomToRemove) - continue - } - k.DeleteConsumerRewardDenom(ctx, denomToRemove) - ctx.EventManager().EmitEvent(sdk.NewEvent( - types.EventTypeRemoveConsumerRewardDenom, - sdk.NewAttribute(types.AttributeConsumerRewardDenom, denomToRemove), - )) - } - return nil -} diff --git a/x/ccv/provider/keeper/msg_server.go b/x/ccv/provider/keeper/msg_server.go index b10cb883c6..439172c5a6 100644 --- a/x/ccv/provider/keeper/msg_server.go +++ b/x/ccv/provider/keeper/msg_server.go @@ -128,16 +128,26 @@ func (k msgServer) RemoveConsumer(goCtx context.Context, msg *types.MsgRemoveCon // ChangeRewardDenoms defines a rpc handler method for MsgChangeRewardDenoms func (k msgServer) ChangeRewardDenoms(goCtx context.Context, msg *types.MsgChangeRewardDenoms) (*types.MsgChangeRewardDenomsResponse, error) { + ctx := sdk.UnwrapSDKContext(goCtx) + if k.GetAuthority() != msg.Authority { return nil, errorsmod.Wrapf(types.ErrUnauthorized, "expected %s, got %s", k.GetAuthority(), msg.Authority) } - sdkCtx := sdk.UnwrapSDKContext(goCtx) - err := k.Keeper.HandleConsumerRewardDenomProposal(sdkCtx, msg) - if err != nil { - return nil, errorsmod.Wrapf(err, "failed handling Change Reward Denoms proposal") + // ValidateBasic is not called automatically for messages from gov proposals + if err := msg.ValidateBasic(); err != nil { + return nil, errorsmod.Wrapf(types.ErrInvalidChangeRewardDenoms, "error: %s", err.Error()) } + eventAttributes := k.Keeper.ChangeRewardDenoms(ctx, msg.DenomsToAdd, msg.DenomsToRemove) + + ctx.EventManager().EmitEvent( + sdk.NewEvent( + types.EventTypeChangeConsumerRewardDenom, + eventAttributes..., + ), + ) + return &types.MsgChangeRewardDenomsResponse{}, nil } diff --git a/x/ccv/provider/keeper/proposal.go b/x/ccv/provider/keeper/proposal.go deleted file mode 100644 index 358aee2936..0000000000 --- a/x/ccv/provider/keeper/proposal.go +++ /dev/null @@ -1,18 +0,0 @@ -package keeper - -import ( - sdk "github.com/cosmos/cosmos-sdk/types" - - "github.com/cosmos/interchain-security/v5/x/ccv/provider/types" -) - -// Wrapper for the new proposal message MsgChangeRewardDenoms -// Will replace legacy handler HandleLegacyConsumerRewardDenomProposal -func (k Keeper) HandleConsumerRewardDenomProposal(ctx sdk.Context, proposal *types.MsgChangeRewardDenoms) error { - p := types.ChangeRewardDenomsProposal{ - DenomsToAdd: proposal.DenomsToAdd, - DenomsToRemove: proposal.DenomsToRemove, - } - - return k.HandleLegacyConsumerRewardDenomProposal(ctx, &p) -} diff --git a/x/ccv/provider/proposal_handler.go b/x/ccv/provider/proposal_handler.go index 01d92d3e9e..ea66146039 100644 --- a/x/ccv/provider/proposal_handler.go +++ b/x/ccv/provider/proposal_handler.go @@ -22,7 +22,7 @@ func NewProviderProposalHandler(k keeper.Keeper) govv1beta1.Handler { case *types.ConsumerRemovalProposal: return nil case *types.ChangeRewardDenomsProposal: - return k.HandleLegacyConsumerRewardDenomProposal(ctx, c) + return nil default: return errorsmod.Wrapf(sdkerrors.ErrUnknownRequest, "unrecognized ccv proposal content type: %T", c) } diff --git a/x/ccv/provider/types/errors.go b/x/ccv/provider/types/errors.go index 57ab5c2a99..3e213fe382 100644 --- a/x/ccv/provider/types/errors.go +++ b/x/ccv/provider/types/errors.go @@ -49,4 +49,5 @@ var ( ErrCannotCreateTopNChain = errorsmod.Register(ModuleName, 41, "cannot create Top N chain outside permissionlessly") ErrCannotPrepareForLaunch = errorsmod.Register(ModuleName, 42, "cannot prepare chain for launch") ErrInvalidStopTime = errorsmod.Register(ModuleName, 43, "invalid stop time") + ErrInvalidChangeRewardDenoms = errorsmod.Register(ModuleName, 44, "invalid change reward denoms message") ) diff --git a/x/ccv/provider/types/events.go b/x/ccv/provider/types/events.go index 982b383f03..d539d695e2 100644 --- a/x/ccv/provider/types/events.go +++ b/x/ccv/provider/types/events.go @@ -4,8 +4,7 @@ package types const ( EventTypeConsumerClientCreated = "consumer_client_created" EventTypeAssignConsumerKey = "assign_consumer_key" - EventTypeAddConsumerRewardDenom = "add_consumer_reward_denom" - EventTypeRemoveConsumerRewardDenom = "remove_consumer_reward_denom" + EventTypeChangeConsumerRewardDenom = "change_consumer_reward_denom" EventTypeExecuteConsumerChainSlash = "execute_consumer_chain_slash" EventTypeSetConsumerCommissionRate = "set_consumer_commission_rate" EventTypeOptIn = "opt_in" @@ -16,7 +15,8 @@ const ( AttributeUnbondingPeriod = "unbonding_period" AttributeProviderValidatorAddress = "provider_validator_address" AttributeConsumerConsensusPubKey = "consumer_consensus_pub_key" - AttributeConsumerRewardDenom = "consumer_reward_denom" + AttributeAddConsumerRewardDenom = "add_consumer_reward_denom" + AttributeRemoveConsumerRewardDenom = "remove_consumer_reward_denom" AttributeConsumerCommissionRate = "consumer_commission_rate" AttributeConsumerId = "consumer_chain_id" ) From bf85ad4dfa175fd637d61230855a6a85f02b76ca Mon Sep 17 00:00:00 2001 From: mpoke Date: Thu, 29 Aug 2024 21:21:08 +0200 Subject: [PATCH 08/13] add TODOs --- x/ccv/provider/keeper/msg_server.go | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/x/ccv/provider/keeper/msg_server.go b/x/ccv/provider/keeper/msg_server.go index 439172c5a6..8f80076422 100644 --- a/x/ccv/provider/keeper/msg_server.go +++ b/x/ccv/provider/keeper/msg_server.go @@ -103,6 +103,8 @@ func (k msgServer) RemoveConsumer(goCtx context.Context, msg *types.MsgRemoveCon return &resp, errorsmod.Wrapf(types.ErrUnauthorized, "expected owner address %s, got %s", ownerAddress, msg.Signer) } + // TODO (PERMISSIONLESS) -- ValidateBasic is not called automatically for messages from gov proposals + phase := k.Keeper.GetConsumerPhase(ctx, consumerId) if phase != types.ConsumerPhase_CONSUMER_PHASE_LAUNCHED { return nil, errorsmod.Wrapf(types.ErrInvalidPhase, @@ -364,7 +366,7 @@ func (k msgServer) CreateConsumer(goCtx context.Context, msg *types.MsgCreateCon "cannot create a Top N chain using the `MsgCreateConsumer` message; use `MsgUpdateConsumer` instead") } - // TODO UpdateAllowlist & UpdateDenylist + // TODO (PERMISSIONLESS) UpdateAllowlist & UpdateDenylist } if err := k.Keeper.SetConsumerPowerShapingParameters(ctx, consumerId, powerShapingParameters); err != nil { return &resp, errorsmod.Wrapf(types.ErrInvalidPowerShapingParameters, @@ -410,6 +412,8 @@ func (k msgServer) UpdateConsumer(goCtx context.Context, msg *types.MsgUpdateCon return &resp, errorsmod.Wrapf(types.ErrUnauthorized, "expected owner address %s, got %s", ownerAddress, msg.Signer) } + // TODO (PERMISSIONLESS) -- ValidateBasic is not called automatically for messages from gov proposals + // The new owner address can be empty, in which case the consumer chain does not change its owner. // However, if the new owner address is not empty, we verify that it's a valid account address. if strings.TrimSpace(msg.NewOwnerAddress) != "" { From 950da7d34ff292ad6ebc29e10f6d25dd148659a7 Mon Sep 17 00:00:00 2001 From: mpoke Date: Thu, 29 Aug 2024 21:34:55 +0200 Subject: [PATCH 09/13] remove proposal_test --- x/ccv/provider/keeper/proposal_test.go | 80 -------------------------- 1 file changed, 80 deletions(-) delete mode 100644 x/ccv/provider/keeper/proposal_test.go diff --git a/x/ccv/provider/keeper/proposal_test.go b/x/ccv/provider/keeper/proposal_test.go deleted file mode 100644 index 2779426c61..0000000000 --- a/x/ccv/provider/keeper/proposal_test.go +++ /dev/null @@ -1,80 +0,0 @@ -package keeper_test - -// -// Initialization sub-protocol related tests of proposal.go -// - -// TODO (PERMISSIONLESS) -// WE DO NOT go by order in permissionless (?) DO WE need to? -// TestGetConsumerRemovalPropsToExecute tests that pending consumer removal proposals -// that are ready to execute are accessed in order by timestamp via the iterator -//func TestGetConsumerRemovalPropsToExecute(t *testing.T) { -// now := time.Now().UTC() -// sampleProp1 := providertypes.ConsumerRemovalProposal{ConsumerId: "chain-2", StopTime: now} -// sampleProp2 := providertypes.ConsumerRemovalProposal{ConsumerId: "chain-1", StopTime: now.Add(time.Hour)} -// sampleProp3 := providertypes.ConsumerRemovalProposal{ConsumerId: "chain-4", StopTime: now.Add(-time.Hour)} -// sampleProp4 := providertypes.ConsumerRemovalProposal{ConsumerId: "chain-3", StopTime: now} -// sampleProp5 := providertypes.ConsumerRemovalProposal{ConsumerId: "chain-1", StopTime: now.Add(2 * time.Hour)} -// -// getExpectedOrder := func(props []providertypes.ConsumerRemovalProposal, accessTime time.Time) []providertypes.ConsumerRemovalProposal { -// expectedOrder := []providertypes.ConsumerRemovalProposal{} -// for _, prop := range props { -// if !accessTime.Before(prop.StopTime) { -// expectedOrder = append(expectedOrder, prop) -// } -// } -// // sorting by SpawnTime.UnixNano() -// sort.Slice(expectedOrder, func(i, j int) bool { -// if expectedOrder[i].StopTime.UTC() == expectedOrder[j].StopTime.UTC() { -// // proposals with same StopTime -// return expectedOrder[i].ConsumerId < expectedOrder[j].ConsumerId -// } -// return expectedOrder[i].StopTime.UTC().Before(expectedOrder[j].StopTime.UTC()) -// }) -// return expectedOrder -// } -// -// testCases := []struct { -// propSubmitOrder []providertypes.ConsumerRemovalProposal -// accessTime time.Time -// }{ -// { -// propSubmitOrder: []providertypes.ConsumerRemovalProposal{ -// sampleProp1, sampleProp2, sampleProp3, sampleProp4, sampleProp5, -// }, -// accessTime: now, -// }, -// { -// propSubmitOrder: []providertypes.ConsumerRemovalProposal{ -// sampleProp3, sampleProp2, sampleProp1, sampleProp5, sampleProp4, -// }, -// accessTime: now.Add(time.Hour), -// }, -// { -// propSubmitOrder: []providertypes.ConsumerRemovalProposal{ -// sampleProp5, sampleProp4, sampleProp3, sampleProp2, sampleProp1, -// }, -// accessTime: now.Add(-2 * time.Hour), -// }, -// { -// propSubmitOrder: []providertypes.ConsumerRemovalProposal{ -// sampleProp5, sampleProp4, sampleProp3, sampleProp2, sampleProp1, -// }, -// accessTime: now.Add(3 * time.Hour), -// }, -// } -// -// for _, tc := range testCases { -// providerKeeper, ctx, ctrl, _ := testkeeper.GetProviderKeeperAndCtx(t, testkeeper.NewInMemKeeperParams(t)) -// defer ctrl.Finish() -// -// expectedOrderedProps := getExpectedOrder(tc.propSubmitOrder, tc.accessTime) -// -// for _, prop := range tc.propSubmitOrder { -// cpProp := prop -// providerKeeper.SetPendingConsumerRemovalProp(ctx, &cpProp) -// } -// propsToExecute := providerKeeper.GetConsumerRemovalPropsToExecute(ctx.WithBlockTime(tc.accessTime)) -// require.Equal(t, expectedOrderedProps, propsToExecute) -// } -//} From 17bf7d4623c7b0de328fe7f8dd8e36abb6169306 Mon Sep 17 00:00:00 2001 From: mpoke Date: Fri, 30 Aug 2024 08:45:46 +0200 Subject: [PATCH 10/13] remove todos --- x/ccv/provider/keeper/msg_server.go | 9 --------- 1 file changed, 9 deletions(-) diff --git a/x/ccv/provider/keeper/msg_server.go b/x/ccv/provider/keeper/msg_server.go index 8f80076422..962575e00d 100644 --- a/x/ccv/provider/keeper/msg_server.go +++ b/x/ccv/provider/keeper/msg_server.go @@ -103,8 +103,6 @@ func (k msgServer) RemoveConsumer(goCtx context.Context, msg *types.MsgRemoveCon return &resp, errorsmod.Wrapf(types.ErrUnauthorized, "expected owner address %s, got %s", ownerAddress, msg.Signer) } - // TODO (PERMISSIONLESS) -- ValidateBasic is not called automatically for messages from gov proposals - phase := k.Keeper.GetConsumerPhase(ctx, consumerId) if phase != types.ConsumerPhase_CONSUMER_PHASE_LAUNCHED { return nil, errorsmod.Wrapf(types.ErrInvalidPhase, @@ -136,11 +134,6 @@ func (k msgServer) ChangeRewardDenoms(goCtx context.Context, msg *types.MsgChang return nil, errorsmod.Wrapf(types.ErrUnauthorized, "expected %s, got %s", k.GetAuthority(), msg.Authority) } - // ValidateBasic is not called automatically for messages from gov proposals - if err := msg.ValidateBasic(); err != nil { - return nil, errorsmod.Wrapf(types.ErrInvalidChangeRewardDenoms, "error: %s", err.Error()) - } - eventAttributes := k.Keeper.ChangeRewardDenoms(ctx, msg.DenomsToAdd, msg.DenomsToRemove) ctx.EventManager().EmitEvent( @@ -412,8 +405,6 @@ func (k msgServer) UpdateConsumer(goCtx context.Context, msg *types.MsgUpdateCon return &resp, errorsmod.Wrapf(types.ErrUnauthorized, "expected owner address %s, got %s", ownerAddress, msg.Signer) } - // TODO (PERMISSIONLESS) -- ValidateBasic is not called automatically for messages from gov proposals - // The new owner address can be empty, in which case the consumer chain does not change its owner. // However, if the new owner address is not empty, we verify that it's a valid account address. if strings.TrimSpace(msg.NewOwnerAddress) != "" { From 9324d6b0b965a2c2bc7ca00b41b1333c0fcb79a6 Mon Sep 17 00:00:00 2001 From: mpoke Date: Fri, 30 Aug 2024 12:18:34 +0200 Subject: [PATCH 11/13] apply review suggestions --- testutil/keeper/unit_test_helpers.go | 2 ++ x/ccv/provider/keeper/distribution.go | 15 ++++++++-- x/ccv/provider/keeper/distribution_test.go | 32 ++++++++++++++++++++++ x/ccv/provider/types/errors.go | 1 - 4 files changed, 47 insertions(+), 3 deletions(-) diff --git a/testutil/keeper/unit_test_helpers.go b/testutil/keeper/unit_test_helpers.go index 181b69ffcc..e1ae2f4b99 100644 --- a/testutil/keeper/unit_test_helpers.go +++ b/testutil/keeper/unit_test_helpers.go @@ -239,7 +239,9 @@ func SetupForStoppingConsumerChain(t *testing.T, ctx sdk.Context, err := providerKeeper.CreateConsumerClient(ctx, consumerId) require.NoError(t, err) + // set the mapping consumer ID <> client ID for the consumer chain providerKeeper.SetConsumerClientId(ctx, consumerId, "clientID") + // set the channel ID for the consumer chain err = providerKeeper.SetConsumerChain(ctx, "channelID") require.NoError(t, err) } diff --git a/x/ccv/provider/keeper/distribution.go b/x/ccv/provider/keeper/distribution.go index 69964708df..5795082844 100644 --- a/x/ccv/provider/keeper/distribution.go +++ b/x/ccv/provider/keeper/distribution.go @@ -364,26 +364,37 @@ func (k Keeper) HandleSetConsumerCommissionRate(ctx sdk.Context, consumerId stri // TODO: this method needs to be tested func (k Keeper) ChangeRewardDenoms(ctx sdk.Context, denomsToAdd, denomsToRemove []string) []sdk.Attribute { + // initialize an empty slice to store event attributes eventAttributes := []sdk.Attribute{} + + // loop through denomsToAdd and add each denomination if it is not already registered for _, denomToAdd := range denomsToAdd { // Log error and move on if one of the denoms is already registered if k.ConsumerRewardDenomExists(ctx, denomToAdd) { - ctx.Logger().Error("denom %s already registered", denomToAdd) + k.Logger(ctx).Info("ChangeRewardDenoms: denom already registered", + "denomToAdd", denomToAdd, + ) continue } k.SetConsumerRewardDenom(ctx, denomToAdd) eventAttributes = append(eventAttributes, sdk.NewAttribute(types.AttributeAddConsumerRewardDenom, denomToAdd)) } + + // loop through denomsToRemove and remove each denomination if it is registered for _, denomToRemove := range denomsToRemove { // Log error and move on if one of the denoms is not registered if !k.ConsumerRewardDenomExists(ctx, denomToRemove) { - ctx.Logger().Error("denom %s not registered", denomToRemove) + k.Logger(ctx).Info("ChangeRewardDenoms: denom not registered", + "denomToRemove", denomToRemove, + ) continue } k.DeleteConsumerRewardDenom(ctx, denomToRemove) eventAttributes = append(eventAttributes, sdk.NewAttribute(types.AttributeRemoveConsumerRewardDenom, denomToRemove)) } + + // return the slice of event attributes return eventAttributes } diff --git a/x/ccv/provider/keeper/distribution_test.go b/x/ccv/provider/keeper/distribution_test.go index 57ff82bc95..c449b0149c 100644 --- a/x/ccv/provider/keeper/distribution_test.go +++ b/x/ccv/provider/keeper/distribution_test.go @@ -296,3 +296,35 @@ func TestIsEligibleForConsumerRewards(t *testing.T) { require.True(t, keeper.IsEligibleForConsumerRewards(ctx.WithBlockHeight(numberOfBlocks+1), 1)) require.False(t, keeper.IsEligibleForConsumerRewards(ctx.WithBlockHeight(numberOfBlocks+1), 2)) } + +func TestChangeRewardDenoms(t *testing.T) { + keeper, ctx, _, _ := testkeeper.GetProviderKeeperAndCtx(t, testkeeper.NewInMemKeeperParams(t)) + + // Test adding a new denomination + denomsToAdd := []string{"denom1"} + denomsToRemove := []string{} + attributes := keeper.ChangeRewardDenoms(ctx, denomsToAdd, denomsToRemove) + + require.Len(t, attributes, 1) + require.Equal(t, providertypes.AttributeAddConsumerRewardDenom, attributes[0].Key) + require.Equal(t, "denom1", attributes[0].Value) + require.True(t, keeper.ConsumerRewardDenomExists(ctx, "denom1")) + + // Test adding a denomination that is already registered + attributes = keeper.ChangeRewardDenoms(ctx, denomsToAdd, denomsToRemove) + require.Len(t, attributes, 0) // No attributes should be returned since the denom is already registered + + // Test removing a registered denomination + denomsToAdd = []string{} + denomsToRemove = []string{"denom1"} + attributes = keeper.ChangeRewardDenoms(ctx, denomsToAdd, denomsToRemove) + + require.Len(t, attributes, 1) + require.Equal(t, providertypes.AttributeRemoveConsumerRewardDenom, attributes[0].Key) + require.Equal(t, "denom1", attributes[0].Value) + require.False(t, keeper.ConsumerRewardDenomExists(ctx, "denom1")) + + // Test removing a denomination that is not registered + attributes = keeper.ChangeRewardDenoms(ctx, denomsToAdd, denomsToRemove) + require.Len(t, attributes, 0) // No attributes should be returned since the denom is not registered +} diff --git a/x/ccv/provider/types/errors.go b/x/ccv/provider/types/errors.go index 3e213fe382..57ab5c2a99 100644 --- a/x/ccv/provider/types/errors.go +++ b/x/ccv/provider/types/errors.go @@ -49,5 +49,4 @@ var ( ErrCannotCreateTopNChain = errorsmod.Register(ModuleName, 41, "cannot create Top N chain outside permissionlessly") ErrCannotPrepareForLaunch = errorsmod.Register(ModuleName, 42, "cannot prepare chain for launch") ErrInvalidStopTime = errorsmod.Register(ModuleName, 43, "invalid stop time") - ErrInvalidChangeRewardDenoms = errorsmod.Register(ModuleName, 44, "invalid change reward denoms message") ) From 5240deb6afbde65f72c4ec0683fc08c6c8ec737e Mon Sep 17 00:00:00 2001 From: mpoke Date: Fri, 30 Aug 2024 14:00:25 +0200 Subject: [PATCH 12/13] apply review suggestions --- x/ccv/provider/keeper/distribution.go | 4 ++-- x/ccv/provider/keeper/keeper.go | 5 ++++- 2 files changed, 6 insertions(+), 3 deletions(-) diff --git a/x/ccv/provider/keeper/distribution.go b/x/ccv/provider/keeper/distribution.go index 5795082844..87a2e5e9e4 100644 --- a/x/ccv/provider/keeper/distribution.go +++ b/x/ccv/provider/keeper/distribution.go @@ -371,7 +371,7 @@ func (k Keeper) ChangeRewardDenoms(ctx sdk.Context, denomsToAdd, denomsToRemove for _, denomToAdd := range denomsToAdd { // Log error and move on if one of the denoms is already registered if k.ConsumerRewardDenomExists(ctx, denomToAdd) { - k.Logger(ctx).Info("ChangeRewardDenoms: denom already registered", + k.Logger(ctx).Error("ChangeRewardDenoms: denom already registered", "denomToAdd", denomToAdd, ) continue @@ -385,7 +385,7 @@ func (k Keeper) ChangeRewardDenoms(ctx sdk.Context, denomsToAdd, denomsToRemove for _, denomToRemove := range denomsToRemove { // Log error and move on if one of the denoms is not registered if !k.ConsumerRewardDenomExists(ctx, denomToRemove) { - k.Logger(ctx).Info("ChangeRewardDenoms: denom not registered", + k.Logger(ctx).Error("ChangeRewardDenoms: denom not registered", "denomToRemove", denomToRemove, ) continue diff --git a/x/ccv/provider/keeper/keeper.go b/x/ccv/provider/keeper/keeper.go index 1e11afb8de..0fc911c8dc 100644 --- a/x/ccv/provider/keeper/keeper.go +++ b/x/ccv/provider/keeper/keeper.go @@ -636,7 +636,9 @@ func (k Keeper) DeletePendingVSCPackets(ctx sdk.Context, consumerId string) { store.Delete(types.PendingVSCsKey(consumerId)) } -// SetConsumerClientId sets the client id for the given consumer id +// SetConsumerClientId sets the client id for the given consumer id. +// Note that the method also stores a reverse index that can be accessed +// by calling GetClientIdToConsumerId. func (k Keeper) SetConsumerClientId(ctx sdk.Context, consumerId, clientId string) { store := ctx.KVStore(k.storeKey) @@ -672,6 +674,7 @@ func (k Keeper) GetClientIdToConsumerId(ctx sdk.Context, clientId string) (strin } // DeleteConsumerClientId removes from the store the client id for the given consumer id. +// Note that the method also removes the reverse index. func (k Keeper) DeleteConsumerClientId(ctx sdk.Context, consumerId string) { store := ctx.KVStore(k.storeKey) From cd0b35f157bab5b6d1d603d12ecd5725128bb3c5 Mon Sep 17 00:00:00 2001 From: mpoke Date: Fri, 30 Aug 2024 16:08:03 +0200 Subject: [PATCH 13/13] fix UTs --- x/ccv/provider/types/legacy_proposal_test.go | 70 ------------------ x/ccv/provider/types/msg_test.go | 74 ++++++++++++++++++++ 2 files changed, 74 insertions(+), 70 deletions(-) diff --git a/x/ccv/provider/types/legacy_proposal_test.go b/x/ccv/provider/types/legacy_proposal_test.go index 180a4978bd..21f7a2da8a 100644 --- a/x/ccv/provider/types/legacy_proposal_test.go +++ b/x/ccv/provider/types/legacy_proposal_test.go @@ -542,76 +542,6 @@ func TestChangeRewardDenomsProposalValidateBasic(t *testing.T) { } } -func TestMsgUpdateConsumerValidateBasic(t *testing.T) { - testCases := []struct { - name string - powerShapingParameters types.PowerShapingParameters - expPass bool - }{ - { - "success", - types.PowerShapingParameters{ - Top_N: 50, - ValidatorsPowerCap: 100, - ValidatorSetCap: 34, - Allowlist: []string{"addr1"}, - Denylist: nil, - MinStake: 0, - AllowInactiveVals: false, - }, - true, - }, - { - "top N is invalid", - types.PowerShapingParameters{ - Top_N: 10, - ValidatorsPowerCap: 0, - ValidatorSetCap: 0, - Allowlist: nil, - Denylist: nil, - }, - false, - }, - { - "validators power cap is invalid", - types.PowerShapingParameters{ - Top_N: 50, - ValidatorsPowerCap: 101, - ValidatorSetCap: 0, - Allowlist: nil, - Denylist: nil, - MinStake: 0, - AllowInactiveVals: false, - }, - false, - }, - { - "valid proposal", - types.PowerShapingParameters{ - Top_N: 54, - ValidatorsPowerCap: 92, - ValidatorSetCap: 0, - Allowlist: []string{"addr1"}, - Denylist: []string{"addr2", "addr3"}, - MinStake: 0, - AllowInactiveVals: false, - }, - true, - }, - } - - for _, tc := range testCases { - // TODO (PERMISSIONLESS) add more tests - msg, _ := types.NewMsgUpdateConsumer("", "0", "cosmos1p3ucd3ptpw902fluyjzhq3ffgq4ntddac9sa3s", nil, nil, &tc.powerShapingParameters) - err := msg.ValidateBasic() - 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) - } - } -} - func TestValidatePSSFeatures(t *testing.T) { require.NoError(t, types.ValidatePSSFeatures(0, 0)) require.NoError(t, types.ValidatePSSFeatures(50, 0)) diff --git a/x/ccv/provider/types/msg_test.go b/x/ccv/provider/types/msg_test.go index 60f7d31675..ecab465ab9 100644 --- a/x/ccv/provider/types/msg_test.go +++ b/x/ccv/provider/types/msg_test.go @@ -362,3 +362,77 @@ func TestValidateByteSlice(t *testing.T) { } } } + +func TestMsgUpdateConsumerValidateBasic(t *testing.T) { + consAddr1 := "cosmosvalcons1qmq08eruchr5sf5s3rwz7djpr5a25f7xw4mceq" + consAddr2 := "cosmosvalcons1nx7n5uh0ztxsynn4sje6eyq2ud6rc6klc96w39" + consAddr3 := "cosmosvalcons1muys5jyqk4xd27e208nym85kn0t4zjcfeu63fe" + + testCases := []struct { + name string + powerShapingParameters types.PowerShapingParameters + expPass bool + }{ + { + "success", + types.PowerShapingParameters{ + Top_N: 50, + ValidatorsPowerCap: 100, + ValidatorSetCap: 34, + Allowlist: []string{consAddr1}, + Denylist: nil, + MinStake: 0, + AllowInactiveVals: false, + }, + true, + }, + { + "top N is invalid", + types.PowerShapingParameters{ + Top_N: 10, + ValidatorsPowerCap: 0, + ValidatorSetCap: 0, + Allowlist: nil, + Denylist: nil, + }, + false, + }, + { + "validators power cap is invalid", + types.PowerShapingParameters{ + Top_N: 50, + ValidatorsPowerCap: 101, + ValidatorSetCap: 0, + Allowlist: nil, + Denylist: nil, + MinStake: 0, + AllowInactiveVals: false, + }, + false, + }, + { + "valid proposal", + types.PowerShapingParameters{ + Top_N: 54, + ValidatorsPowerCap: 92, + ValidatorSetCap: 0, + Allowlist: []string{consAddr1}, + Denylist: []string{consAddr2, consAddr3}, + MinStake: 0, + AllowInactiveVals: false, + }, + true, + }, + } + + for _, tc := range testCases { + // TODO (PERMISSIONLESS) add more tests + msg, _ := types.NewMsgUpdateConsumer("", "0", "cosmos1p3ucd3ptpw902fluyjzhq3ffgq4ntddac9sa3s", nil, nil, &tc.powerShapingParameters) + err := msg.ValidateBasic() + 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) + } + } +}