From 50bbbfedeb82ee2f826569cf03bfb04d5e49e60b Mon Sep 17 00:00:00 2001 From: Marius Poke Date: Mon, 9 Sep 2024 15:11:29 +0200 Subject: [PATCH] fix!: add ConsumeIdsFromTimeQueue to handle consumers at a given time (#2236) * replace GetConsumersReadyToLaunch with ConsumeConsumersReadyToLaunch * add ConsumeIdsFromTimeQueue for both launch and remove * Update x/ccv/provider/keeper/consumer_lifecycle.go Co-authored-by: Simon Noetzlin --------- Co-authored-by: Simon Noetzlin --- x/ccv/provider/keeper/consumer_lifecycle.go | 211 ++++++++---------- .../keeper/consumer_lifecycle_test.go | 194 +++++++++++----- x/ccv/provider/keeper/distribution_test.go | 4 +- x/ccv/provider/keeper/relay_test.go | 9 +- x/ccv/provider/module.go | 8 +- x/ccv/provider/types/keys.go | 4 +- 6 files changed, 243 insertions(+), 187 deletions(-) diff --git a/x/ccv/provider/keeper/consumer_lifecycle.go b/x/ccv/provider/keeper/consumer_lifecycle.go index 19468c6294..bf09f7210f 100644 --- a/x/ccv/provider/keeper/consumer_lifecycle.go +++ b/x/ccv/provider/keeper/consumer_lifecycle.go @@ -61,26 +61,19 @@ func (k Keeper) InitializeConsumer(ctx sdk.Context, consumerId string) (time.Tim } // 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) { - initializationParameters, 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, initializationParameters.SpawnTime) - if err != nil { - ctx.Logger().Error("could not remove consumer from to-be-launched queue", - "consumerId", consumerId, - "error", err) - continue - } - +func (k Keeper) BeginBlockLaunchConsumers(ctx sdk.Context) error { + consumerIds, err := k.ConsumeIdsFromTimeQueue( + ctx, + types.SpawnTimeToConsumerIdsKeyPrefix(), + k.GetConsumersToBeLaunched, + k.DeleteAllConsumersToBeLaunched, + k.AppendConsumerToBeLaunched, + 200, + ) + if err != nil { + return errorsmod.Wrapf(ccv.ErrInvalidConsumerState, "getting consumers ready to laumch: %s", err.Error()) + } + for _, consumerId := range consumerIds { cachedCtx, writeFn := ctx.CacheContext() err = k.LaunchConsumer(cachedCtx, consumerId) if err != nil { @@ -94,49 +87,77 @@ func (k Keeper) BeginBlockLaunchConsumers(ctx sdk.Context) { ctx.EventManager().EmitEvents(cachedCtx.EventManager().Events()) writeFn() } + 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 { +// ConsumeIdsFromTimeQueue returns from a time queue the consumer ids for which the associated time passed. +// The number of ids return is limited to 'limit'. The ids returned are removed from the time queue. +func (k Keeper) ConsumeIdsFromTimeQueue( + ctx sdk.Context, + timeQueueKeyPrefix byte, + getIds func(sdk.Context, time.Time) (types.ConsumerIds, error), + deleteAllIds func(sdk.Context, time.Time), + appendId func(sdk.Context, string, time.Time) error, + limit int, +) ([]string, error) { store := ctx.KVStore(k.storeKey) - spawnTimeToConsumerIdsKeyPrefix := types.SpawnTimeToConsumerIdsKeyPrefix() - iterator := storetypes.KVStorePrefixIterator(store, []byte{spawnTimeToConsumerIdsKeyPrefix}) - defer iterator.Close() - result := []string{} + nextTime := []string{} + timestampsToDelete := []time.Time{} + + iterator := storetypes.KVStorePrefixIterator(store, []byte{timeQueueKeyPrefix}) + defer iterator.Close() for ; iterator.Valid(); iterator.Next() { - spawnTime, err := types.ParseTime(spawnTimeToConsumerIdsKeyPrefix, iterator.Key()) + if len(result) >= limit { + break + } + ts, err := types.ParseTime(timeQueueKeyPrefix, iterator.Key()) if err != nil { - k.Logger(ctx).Error("failed to parse spawn time", - "error", err) - continue + return result, fmt.Errorf("parsing removal time: %w", err) } - if spawnTime.After(ctx.BlockTime()) { - return result + if ts.After(ctx.BlockTime()) { + break } - // 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) + consumerIds, err := getIds(ctx, ts) if err != nil { - k.Logger(ctx).Error("failed to retrieve consumers to launch", - "spawn time", spawnTime, - "error", err) - continue + return result, + fmt.Errorf("getting consumers ids, ts(%s): %w", ts.String(), err) } - 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 { + + timestampsToDelete = append(timestampsToDelete, ts) + + availableSlots := limit - len(result) + if availableSlots >= len(consumerIds.Ids) { + // consumer all the ids result = append(result, consumerIds.Ids...) + } else { + // consume only availableSlots + result = append(result, consumerIds.Ids[:availableSlots]...) + // and leave the others for next time + nextTime = consumerIds.Ids[availableSlots:] + break } } - return result + // remove consumers to prevent handling them twice + for i, ts := range timestampsToDelete { + deleteAllIds(ctx, ts) + if i == len(timestampsToDelete)-1 { + // for the last ts consumed, store back the ids for later + for _, consumerId := range nextTime { + err := appendId(ctx, consumerId, ts) + if err != nil { + return result, + fmt.Errorf("failed to append consumer id, consumerId(%s), ts(%s): %w", + consumerId, ts.String(), err) + } + } + } + } + + return result, nil } // LaunchConsumer launches the chain with the provided consumer id by creating the consumer client and the respective @@ -379,26 +400,19 @@ func (k Keeper) StopAndPrepareForConsumerRemoval(ctx sdk.Context, consumerId str } // BeginBlockRemoveConsumers iterates over the pending consumer proposals and stop/removes the chain if the removal time has passed -func (k Keeper) BeginBlockRemoveConsumers(ctx sdk.Context) { - // TODO (PERMISSIONLESS): parameterize the limit - for _, consumerId := range k.GetConsumersReadyToStop(ctx, 200) { - removalTime, err := k.GetConsumerRemovalTime(ctx, consumerId) - if err != nil { - k.Logger(ctx).Error("chain could not be stopped", - "consumerId", consumerId, - "error", err.Error()) - continue - } - - // Remove consumer to prevent re-trying removing this chain. - err = k.RemoveConsumerToBeRemoved(ctx, consumerId, removalTime) - if err != nil { - ctx.Logger().Error("could not remove consumer from to-be-removed queue", - "consumerId", consumerId, - "error", err) - continue - } - +func (k Keeper) BeginBlockRemoveConsumers(ctx sdk.Context) error { + consumerIds, err := k.ConsumeIdsFromTimeQueue( + ctx, + types.RemovalTimeToConsumerIdsKeyPrefix(), + k.GetConsumersToBeRemoved, + k.DeleteAllConsumersToBeRemoved, + k.AppendConsumerToBeRemoved, + 200, + ) + if err != nil { + return errorsmod.Wrapf(ccv.ErrInvalidConsumerState, "getting consumers ready to stop: %s", err.Error()) + } + for _, consumerId := range consumerIds { // delete consumer chain in a cached context to abort deletion in case of errors cachedCtx, writeFn := ctx.CacheContext() err = k.DeleteConsumerChain(cachedCtx, consumerId) @@ -409,58 +423,11 @@ func (k Keeper) BeginBlockRemoveConsumers(ctx sdk.Context) { continue } - // The cached context is created with a new EventManager so we merge the event into the original context + // 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 deletion", - "consumer id", consumerId, - "removal time", removalTime, - ) - } -} - -// 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) - - removalTimeToConsumerIdsKeyPrefix := types.RemovalTimeToConsumerIdsKeyPrefix() - iterator := storetypes.KVStorePrefixIterator(store, []byte{removalTimeToConsumerIdsKeyPrefix}) - defer iterator.Close() - - result := []string{} - for ; iterator.Valid(); iterator.Next() { - removalTime, err := types.ParseTime(removalTimeToConsumerIdsKeyPrefix, iterator.Key()) - if err != nil { - k.Logger(ctx).Error("failed to parse removal time", - "error", err) - continue - } - if removalTime.After(ctx.BlockTime()) { - return result - } - - consumers, err := k.GetConsumersToBeRemoved(ctx, removalTime) - if err != nil { - k.Logger(ctx).Error("failed to retrieve consumers to remove", - "removal time", removalTime, - "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 + return nil } // DeleteConsumerChain cleans up the state of the given consumer chain @@ -657,6 +624,12 @@ func (k Keeper) RemoveConsumerToBeLaunched(ctx sdk.Context, consumerId string, s return k.removeConsumerIdFromTime(ctx, consumerId, types.SpawnTimeToConsumerIdsKey, spawnTime) } +// DeleteAllConsumersToBeLaunched deletes all consumer to be launched at this specific spawn time +func (k Keeper) DeleteAllConsumersToBeLaunched(ctx sdk.Context, spawnTime time.Time) { + store := ctx.KVStore(k.storeKey) + store.Delete(types.SpawnTimeToConsumerIdsKey(spawnTime)) +} + // GetConsumersToBeRemoved returns all the consumer ids of chains stored under this removal time func (k Keeper) GetConsumersToBeRemoved(ctx sdk.Context, removalTime time.Time) (types.ConsumerIds, error) { return k.getConsumerIdsBasedOnTime(ctx, types.RemovalTimeToConsumerIdsKey, removalTime) @@ -671,3 +644,9 @@ func (k Keeper) AppendConsumerToBeRemoved(ctx sdk.Context, consumerId string, re func (k Keeper) RemoveConsumerToBeRemoved(ctx sdk.Context, consumerId string, removalTime time.Time) error { return k.removeConsumerIdFromTime(ctx, consumerId, types.RemovalTimeToConsumerIdsKey, removalTime) } + +// DeleteAllConsumersToBeRemoved deletes all consumer to be removed at this specific removal time +func (k Keeper) DeleteAllConsumersToBeRemoved(ctx sdk.Context, removalTime time.Time) { + store := ctx.KVStore(k.storeKey) + store.Delete(types.RemovalTimeToConsumerIdsKey(removalTime)) +} diff --git a/x/ccv/provider/keeper/consumer_lifecycle_test.go b/x/ccv/provider/keeper/consumer_lifecycle_test.go index cadedbf9df..a412b8826c 100644 --- a/x/ccv/provider/keeper/consumer_lifecycle_test.go +++ b/x/ccv/provider/keeper/consumer_lifecycle_test.go @@ -316,7 +316,8 @@ func TestBeginBlockLaunchConsumers(t *testing.T) { providerKeeper.SetOptedIn(ctx, "3", providertypes.NewProviderConsAddress(consAddr)) - providerKeeper.BeginBlockLaunchConsumers(ctx) + err := providerKeeper.BeginBlockLaunchConsumers(ctx) + require.NoError(t, err) // first chain was successfully launched phase := providerKeeper.GetConsumerPhase(ctx, "0") @@ -351,41 +352,140 @@ func TestBeginBlockLaunchConsumers(t *testing.T) { 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)) +func TestConsumeIdsFromTimeQueue(t *testing.T) { + expectedConsumerIds := []string{"1", "2", "3", "4"} + timestamps := []time.Time{time.Unix(10, 0), time.Unix(20, 0), 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) + testCases := []struct { + name string + ts time.Time + limit int + expOutcome func(sdk.Context, []string, func(sdk.Context, time.Time) (providertypes.ConsumerIds, error)) + }{ + { + name: "timestamp too early", + ts: time.Unix(9, 999999999), + limit: 3, + expOutcome: func(ctx sdk.Context, ids []string, getIds func(sdk.Context, time.Time) (providertypes.ConsumerIds, error)) { + require.Empty(t, ids) + }, + }, + { + name: "first timestamp", + ts: timestamps[0], + limit: 2, + expOutcome: func(ctx sdk.Context, ids []string, getIds func(sdk.Context, time.Time) (providertypes.ConsumerIds, error)) { + require.Equal(t, expectedConsumerIds[0:2], ids) + + // check that all consumers where removed + consumerIds, err := getIds(ctx, timestamps[0]) + require.NoError(t, err) + require.Empty(t, consumerIds) + }, + }, + { + name: "first timestamp, with limit", + ts: timestamps[0], + limit: 1, + expOutcome: func(ctx sdk.Context, ids []string, getIds func(sdk.Context, time.Time) (providertypes.ConsumerIds, error)) { + require.Equal(t, expectedConsumerIds[0:1], ids) + + // second consumer remained + ret, err := getIds(ctx, timestamps[0]) + require.NoError(t, err) + require.Equal(t, providertypes.ConsumerIds{ + Ids: []string{expectedConsumerIds[1]}, + }, ret) + }, + }, + { + name: "second timestamp", + ts: timestamps[1], + limit: 3, + expOutcome: func(ctx sdk.Context, ids []string, getIds func(sdk.Context, time.Time) (providertypes.ConsumerIds, error)) { + require.Equal(t, expectedConsumerIds[0:3], ids) + + // check that all consumers where removed + ret, err := getIds(ctx, timestamps[0]) + require.NoError(t, err) + require.Empty(t, ret) + ret, err = getIds(ctx, timestamps[1]) + require.NoError(t, err) + require.Empty(t, ret) + }, + }, + { + name: "third timestamp, with limit", + ts: timestamps[1], + limit: 3, + expOutcome: func(ctx sdk.Context, ids []string, getIds func(sdk.Context, time.Time) (providertypes.ConsumerIds, error)) { + require.Equal(t, expectedConsumerIds[0:3], ids) + + // 4th consumer remained + ret, err := getIds(ctx, timestamps[0]) + require.NoError(t, err) + require.Empty(t, ret) + ret, err = getIds(ctx, timestamps[1]) + require.NoError(t, err) + require.Empty(t, ret) + ret, err = getIds(ctx, timestamps[2]) + require.NoError(t, err) + require.Equal(t, providertypes.ConsumerIds{ + Ids: []string{expectedConsumerIds[3]}, + }, ret) + }, + }, + } - // time has not yet reached the spawn time of "consumerId1" - ctx = ctx.WithBlockTime(time.Unix(9, 999999999)) - require.Empty(t, providerKeeper.GetConsumersReadyToLaunch(ctx, 3)) + // test for consumers to be launched + for _, tc := range testCases { + providerKeeper, ctx, ctrl, _ := testkeeper.GetProviderKeeperAndCtx(t, testkeeper.NewInMemKeeperParams(t)) + defer ctrl.Finish() - // time has reached the spawn time of "consumerId1" - ctx = ctx.WithBlockTime(time.Unix(10, 0)) - require.Equal(t, []string{"consumerId1"}, providerKeeper.GetConsumersReadyToLaunch(ctx, 3)) + callCases := []struct { + timeQueueKeyPrefix byte + getIds func(sdk.Context, time.Time) (providertypes.ConsumerIds, error) + deleteAllIds func(sdk.Context, time.Time) + appendId func(sdk.Context, string, time.Time) error + }{ + { + timeQueueKeyPrefix: providertypes.SpawnTimeToConsumerIdsKeyPrefix(), + getIds: providerKeeper.GetConsumersToBeLaunched, + deleteAllIds: providerKeeper.DeleteAllConsumersToBeLaunched, + appendId: providerKeeper.AppendConsumerToBeLaunched, + }, + { + timeQueueKeyPrefix: providertypes.RemovalTimeToConsumerIdsKeyPrefix(), + getIds: providerKeeper.GetConsumersToBeRemoved, + deleteAllIds: providerKeeper.DeleteAllConsumersToBeRemoved, + appendId: providerKeeper.AppendConsumerToBeRemoved, + }, + } + for _, cc := range callCases { + err := cc.appendId(ctx, expectedConsumerIds[0], timestamps[0]) + require.NoError(t, err) + err = cc.appendId(ctx, expectedConsumerIds[1], timestamps[0]) + require.NoError(t, err) + err = cc.appendId(ctx, expectedConsumerIds[2], timestamps[1]) + require.NoError(t, err) + err = cc.appendId(ctx, expectedConsumerIds[3], timestamps[2]) + require.NoError(t, err) - // 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)) + ctx = ctx.WithBlockTime(tc.ts) - // 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)) + consumerIds, err := providerKeeper.ConsumeIdsFromTimeQueue( + ctx, + cc.timeQueueKeyPrefix, + cc.getIds, + cc.deleteAllIds, + cc.appendId, + tc.limit, + ) + require.NoError(t, err) - // 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)) + tc.expOutcome(ctx, consumerIds, cc.getIds) + } + } } // Tests the CreateConsumerClient method against the spec, @@ -751,7 +851,8 @@ func TestBeginBlockStopConsumers(t *testing.T) { // Test execution // - providerKeeper.BeginBlockRemoveConsumers(ctx) + err = providerKeeper.BeginBlockRemoveConsumers(ctx) + require.NoError(t, err) // Only the 3rd (final) proposal is still stored as pending phase := providerKeeper.GetConsumerPhase(ctx, consumerIds[0]) @@ -763,37 +864,6 @@ func TestBeginBlockStopConsumers(t *testing.T) { require.Equal(t, providertypes.CONSUMER_PHASE_STOPPED, 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.AppendConsumerToBeRemoved(ctx, "consumerId1", time.Unix(10, 0)) - require.NoError(t, err) - err = providerKeeper.AppendConsumerToBeRemoved(ctx, "consumerId2", time.Unix(20, 0)) - require.NoError(t, err) - err = providerKeeper.AppendConsumerToBeRemoved(ctx, "consumerId3", time.Unix(30, 0)) - require.NoError(t, err) - - // time has not yet reached the removal time of "consumerId1" - ctx = ctx.WithBlockTime(time.Unix(9, 999999999)) - require.Empty(t, providerKeeper.GetConsumersReadyToStop(ctx, 3)) - - // time has reached the removal time of "consumerId1" - ctx = ctx.WithBlockTime(time.Unix(10, 0)) - require.Equal(t, []string{"consumerId1"}, providerKeeper.GetConsumersReadyToStop(ctx, 3)) - - // time has reached the removal 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 removal time of all chains - ctx = ctx.WithBlockTime(time.Unix(30, 0)) - require.Equal(t, []string{"consumerId1", "consumerId2", "consumerId3"}, providerKeeper.GetConsumersReadyToStop(ctx, 3)) -} - // Tests the DeleteConsumerChain 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 diff --git a/x/ccv/provider/keeper/distribution_test.go b/x/ccv/provider/keeper/distribution_test.go index d9ff18ac75..1fa48eb49f 100644 --- a/x/ccv/provider/keeper/distribution_test.go +++ b/x/ccv/provider/keeper/distribution_test.go @@ -38,7 +38,7 @@ func TestComputeConsumerTotalVotingPower(t *testing.T) { return *val } - chainID := "consumer" + chainID := CONSUMER_CHAIN_ID expTotalPower := int64(0) // verify that the total power returned is equal to zero @@ -76,7 +76,7 @@ func TestComputeConsumerTotalVotingPower(t *testing.T) { func TestIdentifyConsumerChainIDFromIBCPacket(t *testing.T) { var ( - chainID = "consumer" + chainID = CONSUMER_CHAIN_ID ccvChannel = "channel-0" ) diff --git a/x/ccv/provider/keeper/relay_test.go b/x/ccv/provider/keeper/relay_test.go index 3a6c1bc757..7923056fe0 100644 --- a/x/ccv/provider/keeper/relay_test.go +++ b/x/ccv/provider/keeper/relay_test.go @@ -532,7 +532,8 @@ func TestSendVSCPacketsToChainFailure(t *testing.T) { // Increase the block time by `unbondingTime` so the chain actually gets deleted ctx = ctx.WithBlockTime(ctx.BlockTime().Add(unbondingTime)) - providerKeeper.BeginBlockRemoveConsumers(ctx) + err = providerKeeper.BeginBlockRemoveConsumers(ctx) + require.NoError(t, err) // Pending VSC packets should be deleted in DeleteConsumerChain require.Empty(t, providerKeeper.GetPendingVSCPackets(ctx, CONSUMER_ID)) @@ -583,7 +584,8 @@ func TestOnTimeoutPacketStopsChain(t *testing.T) { // increase the block time by `unbondingTime` so the chain actually gets deleted ctx = ctx.WithBlockTime(ctx.BlockTime().Add(unbondingTime)) - providerKeeper.BeginBlockRemoveConsumers(ctx) + err = providerKeeper.BeginBlockRemoveConsumers(ctx) + require.NoError(t, err) testkeeper.TestProviderStateIsCleanedAfterConsumerChainIsDeleted(t, ctx, providerKeeper, CONSUMER_ID, "channelID", false) } @@ -635,7 +637,8 @@ func TestOnAcknowledgementPacketWithAckError(t *testing.T) { // increase the block time by `unbondingTime` so the chain actually gets deleted ctx = ctx.WithBlockTime(ctx.BlockTime().Add(unbondingTime)) - providerKeeper.BeginBlockRemoveConsumers(ctx) + err = providerKeeper.BeginBlockRemoveConsumers(ctx) + require.NoError(t, err) testkeeper.TestProviderStateIsCleanedAfterConsumerChainIsDeleted(t, ctx, providerKeeper, CONSUMER_ID, "channelID", false) } diff --git a/x/ccv/provider/module.go b/x/ccv/provider/module.go index 487cffbefc..027fd2ac72 100644 --- a/x/ccv/provider/module.go +++ b/x/ccv/provider/module.go @@ -175,9 +175,13 @@ 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.BeginBlockLaunchConsumers(sdkCtx) + if err := am.keeper.BeginBlockLaunchConsumers(sdkCtx); err != nil { + return err + } // Stop and remove state for any consumer chains that are due to be stopped - am.keeper.BeginBlockRemoveConsumers(sdkCtx) + if err := am.keeper.BeginBlockRemoveConsumers(sdkCtx); err != nil { + return err + } // 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 diff --git a/x/ccv/provider/types/keys.go b/x/ccv/provider/types/keys.go index c10f0a1cb9..62869dcebf 100644 --- a/x/ccv/provider/types/keys.go +++ b/x/ccv/provider/types/keys.go @@ -719,12 +719,12 @@ func RemovalTimeToConsumerIdsKeyPrefix() byte { // RemovalTimeToConsumerIdsKey returns the key prefix for storing the removal times of consumer chains // that are about to be removed -func RemovalTimeToConsumerIdsKey(spawnTime time.Time) []byte { +func RemovalTimeToConsumerIdsKey(removalTime time.Time) []byte { return ccvtypes.AppendMany( // append the prefix []byte{RemovalTimeToConsumerIdsKeyPrefix()}, // append the time - sdk.FormatTimeBytes(spawnTime), + sdk.FormatTimeBytes(removalTime), ) }