From 28c44459138fce264ccd79876825d88074531ad7 Mon Sep 17 00:00:00 2001 From: insumity Date: Mon, 5 Aug 2024 18:13:35 +0300 Subject: [PATCH] removed (partially) consumer removal proposal --- x/ccv/provider/keeper/msg_server.go | 6 +- x/ccv/provider/keeper/permissionless.go | 77 +++++- x/ccv/provider/keeper/permissionless_test.go | 54 ++++ x/ccv/provider/keeper/proposal.go | 112 ++++---- x/ccv/provider/keeper/proposal_test.go | 261 ++++++++----------- x/ccv/provider/module.go | 5 +- x/ccv/provider/proposal_handler.go | 2 +- x/ccv/provider/types/keys.go | 18 +- x/ccv/provider/types/keys_test.go | 5 +- x/ccv/provider/types/msg.go | 1 + 10 files changed, 305 insertions(+), 236 deletions(-) diff --git a/x/ccv/provider/keeper/msg_server.go b/x/ccv/provider/keeper/msg_server.go index 8bcec5ae5b..ea8aa6ee01 100644 --- a/x/ccv/provider/keeper/msg_server.go +++ b/x/ccv/provider/keeper/msg_server.go @@ -97,10 +97,8 @@ func (k msgServer) RemoveConsumer( } ctx := sdk.UnwrapSDKContext(goCtx) - err := k.Keeper.HandleConsumerRemovalProposal(ctx, msg) - if err != nil { - return nil, errorsmod.Wrapf(err, "failed handling ConsumerAddition proposal") - } + + k.Keeper.SetConsumerIdToStopTime(ctx, msg.ConsumerId, msg.StopTime) return &types.MsgRemoveConsumerResponse{}, nil } diff --git a/x/ccv/provider/keeper/permissionless.go b/x/ccv/provider/keeper/permissionless.go index eda61b8f03..120bdc9a84 100644 --- a/x/ccv/provider/keeper/permissionless.go +++ b/x/ccv/provider/keeper/permissionless.go @@ -7,6 +7,7 @@ import ( "fmt" sdk "github.com/cosmos/cosmos-sdk/types" "github.com/cosmos/interchain-security/v5/x/ccv/provider/types" + "time" ) // ConsumerPhase captures the phases of a consumer chain according to `docs/docs/adrs/adr-018-permissionless-ics.md` @@ -198,7 +199,7 @@ func (k Keeper) GetConsumerIdToPhase(ctx sdk.Context, consumerId string) (Consum } // SetConsumerIdToPhase sets the phase associated with this consumer id -// TODO (PERMISSIONLESS): use this method when launching and when stopping a chian +// TODO (PERMISSIONLESS): use this method when launching and when stopping a chain func (k Keeper) SetConsumerIdToPhase(ctx sdk.Context, consumerId string, phase ConsumerPhase) { store := ctx.KVStore(k.storeKey) store.Set(types.ConsumerIdToPhaseKey(consumerId), []byte{byte(phase)}) @@ -210,6 +211,36 @@ func (k Keeper) DeleteConsumerIdToPhase(ctx sdk.Context, consumerId string) { store.Delete(types.ConsumerIdToPhaseKey(consumerId)) } +// GetConsumerIdToStopTime returns the stop time associated with the to-be-stopped chain with consumer id +func (k Keeper) GetConsumerIdToStopTime(ctx sdk.Context, consumerId string) (time.Time, bool) { + store := ctx.KVStore(k.storeKey) + buf := store.Get(types.ConsumerIdToStopTimeKey(consumerId)) + if buf == nil { + return time.Time{}, false + } + var time time.Time + if err := time.UnmarshalBinary(buf); err != nil { + panic(fmt.Errorf("failed to unmarshal time: %w", err)) + } + return time, true +} + +// SetConsumerIdToStopTime sets the stop time associated with this consumer id +func (k Keeper) SetConsumerIdToStopTime(ctx sdk.Context, consumerId string, stopTime time.Time) { + store := ctx.KVStore(k.storeKey) + buf, err := stopTime.MarshalBinary() + if err != nil { + panic(fmt.Errorf("failed to marshal time: %w", err)) + } + store.Set(types.ConsumerIdToStopTimeKey(consumerId), buf) +} + +// DeleteConsumerIdToStopTime deletes the stop time associated with this consumer id +func (k Keeper) DeleteConsumerIdToStopTime(ctx sdk.Context, consumerId string) { + store := ctx.KVStore(k.storeKey) + store.Delete(types.ConsumerIdToStopTimeKey(consumerId)) +} + // 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) @@ -242,16 +273,20 @@ func (k Keeper) GetInitializedConsumersReadyToLaunch(ctx sdk.Context) []string { var consumerIds []string for ; iterator.Valid(); iterator.Next() { + // the `consumerId` resides in the whole key, but we skip the first byte (because it's the `ConsumerIdKey`) + // plus 8 more bytes for the `uint64` in the key that contains the length of the `consumerId` + consumerId := string(iterator.Key()[1+8:]) + var record types.ConsumerInitializationRecord err := record.Unmarshal(iterator.Value()) if err != nil { - panic(fmt.Errorf("failed to unmarshal consumer record: %w for consumer id: %s", err, string(iterator.Value()))) + panic(fmt.Errorf("failed to unmarshal consumer record: %w for consumer id: %s", err, consumerId)) } - if !ctx.BlockTime().Before(record.SpawnTime) { - // the `consumerId` resides in the whole key, but we skip the first byte (because it's the `ConsumerIdKey`) - // plus 8 more bytes for the `uint64` in the key that contains the length of the `consumerId` - consumerIds = append(consumerIds, string(iterator.Key()[1+8:])) + // if current block time is equal to or after spawnTime, and the chain is initialized, we can launch the chain + phase, found := k.GetConsumerIdToPhase(ctx, consumerId) + if !ctx.BlockTime().Before(record.SpawnTime) && (found && phase == Initialized) { + consumerIds = append(consumerIds, consumerId) } } @@ -344,3 +379,33 @@ func (k Keeper) UpdateConsumer(ctx sdk.Context, consumerId string) error { return nil } + +// GetLaunchedConsumersReadyToStop returns the consumer ids of the pending launched consumer chains +// that are ready to stop +func (k Keeper) GetLaunchedConsumersReadyToStop(ctx sdk.Context) []string { + store := ctx.KVStore(k.storeKey) + iterator := storetypes.KVStorePrefixIterator(store, types.ConsumerIdToStopTimeKeyNamePrefix()) + defer iterator.Close() + + var consumerIds []string + + for ; iterator.Valid(); iterator.Next() { + // the `consumerId` resides in the whole key, but we skip the first byte (because it's the `ConsumerIdKey`) + // plus 8 more bytes for the `uint64` in the key that contains the length of the `consumerId` + consumerId := string(iterator.Key()[1+8:]) + + var stopTime time.Time + err := stopTime.UnmarshalBinary(iterator.Value()) + if err != nil { + panic(fmt.Errorf("failed to unmarshal stop stopTime: %w for consumer id: %s", err, consumerId)) + } + + // if current block time is equal to or after stop stopTime, and the chain is launched we can stop the chain + phase, found := k.GetConsumerIdToPhase(ctx, consumerId) + if !ctx.BlockTime().Before(stopTime) && (found && phase == Launched) { + consumerIds = append(consumerIds, string(iterator.Key()[1+8:])) + } + } + + return consumerIds +} diff --git a/x/ccv/provider/keeper/permissionless_test.go b/x/ccv/provider/keeper/permissionless_test.go index a45d3c6c65..4aaefe785b 100644 --- a/x/ccv/provider/keeper/permissionless_test.go +++ b/x/ccv/provider/keeper/permissionless_test.go @@ -187,6 +187,25 @@ func TestConsumerIdToPhase(t *testing.T) { require.Equal(t, keeper.Launched, phase) } +// TestConsumerIdToStopTime tests the getter, setter, and deletion methods of the consumer id to stop times +func TestConsumerIdToStopTime(t *testing.T) { + providerKeeper, ctx, ctrl, _ := testkeeper.GetProviderKeeperAndCtx(t, testkeeper.NewInMemKeeperParams(t)) + defer ctrl.Finish() + + _, found := providerKeeper.GetConsumerIdToStopTime(ctx, "consumerId") + require.False(t, found) + + expectedStopTime := time.Unix(1234, 56789) + providerKeeper.SetConsumerIdToStopTime(ctx, "consumerId", expectedStopTime) + actualStopTime, found := providerKeeper.GetConsumerIdToStopTime(ctx, "consumerId") + require.True(t, found) + require.Equal(t, actualStopTime, expectedStopTime) + + providerKeeper.DeleteConsumerIdToStopTime(ctx, "consumerId") + _, found = providerKeeper.GetConsumerIdToStopTime(ctx, "consumerId") + require.False(t, found) +} + // 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)) @@ -196,10 +215,13 @@ func TestGetInitializedConsumersReadyToLaunch(t *testing.T) { require.Empty(t, providerKeeper.GetInitializedConsumersReadyToLaunch(ctx)) // set 3 initialization records with different spawn times + providerKeeper.SetConsumerIdToPhase(ctx, "consumerId1", keeper.Initialized) providerKeeper.SetConsumerIdToInitializationRecord(ctx, "consumerId1", providertypes.ConsumerInitializationRecord{SpawnTime: time.Unix(10, 0)}) + providerKeeper.SetConsumerIdToPhase(ctx, "consumerId2", keeper.Initialized) providerKeeper.SetConsumerIdToInitializationRecord(ctx, "consumerId2", providertypes.ConsumerInitializationRecord{SpawnTime: time.Unix(20, 0)}) + providerKeeper.SetConsumerIdToPhase(ctx, "consumerId3", keeper.Initialized) providerKeeper.SetConsumerIdToInitializationRecord(ctx, "consumerId3", providertypes.ConsumerInitializationRecord{SpawnTime: time.Unix(30, 0)}) @@ -219,3 +241,35 @@ func TestGetInitializedConsumersReadyToLaunch(t *testing.T) { ctx = ctx.WithBlockTime(time.Unix(30, 0)) require.Equal(t, []string{"consumerId1", "consumerId2", "consumerId3"}, providerKeeper.GetInitializedConsumersReadyToLaunch(ctx)) } + +func TestGetLaunchedConsumersReadyToStop(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)) + + // set 3 initialization records with different spawn times + providerKeeper.SetConsumerIdToPhase(ctx, "consumerId1", keeper.Launched) + providerKeeper.SetConsumerIdToStopTime(ctx, "consumerId1", time.Unix(10, 0)) + providerKeeper.SetConsumerIdToPhase(ctx, "consumerId2", keeper.Launched) + providerKeeper.SetConsumerIdToStopTime(ctx, "consumerId2", time.Unix(20, 0)) + providerKeeper.SetConsumerIdToPhase(ctx, "consumerId3", keeper.Launched) + providerKeeper.SetConsumerIdToStopTime(ctx, "consumerId3", time.Unix(30, 0)) + + // time has not yet reached the stop time of "consumerId1" + ctx = ctx.WithBlockTime(time.Unix(9, 999999999)) + require.Empty(t, providerKeeper.GetLaunchedConsumersReadyToStop(ctx)) + + // time has reached the stop time of "consumerId1" + ctx = ctx.WithBlockTime(time.Unix(10, 0)) + require.Equal(t, []string{"consumerId1"}, providerKeeper.GetLaunchedConsumersReadyToStop(ctx)) + + // 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)) + + // 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)) +} diff --git a/x/ccv/provider/keeper/proposal.go b/x/ccv/provider/keeper/proposal.go index 89acbdaf0e..a4199e1dc5 100644 --- a/x/ccv/provider/keeper/proposal.go +++ b/x/ccv/provider/keeper/proposal.go @@ -23,17 +23,6 @@ import ( ccv "github.com/cosmos/interchain-security/v5/x/ccv/types" ) -// Wrapper for the new proposal message MsgConsumerRemoval -// Will replace legacy handler HandleLegacyConsumerRemovalProposal -func (k Keeper) HandleConsumerRemovalProposal(ctx sdk.Context, proposal *types.MsgRemoveConsumer) error { - p := types.ConsumerRemovalProposal{ - ConsumerId: proposal.ConsumerId, - StopTime: proposal.StopTime, - } - - return k.HandleLegacyConsumerRemovalProposal(ctx, &p) -} - // Wrapper for the new proposal message MsgChangeRewardDenoms // Will replace legacy handler HandleLegacyConsumerRewardDenomProposal func (k Keeper) HandleConsumerRewardDenomProposal(ctx sdk.Context, proposal *types.MsgChangeRewardDenoms) error { @@ -441,73 +430,72 @@ func (k Keeper) DeletePendingConsumerRemovalProps(ctx sdk.Context, proposals ... } } -// BeginBlockCCR iterates over the pending consumer removal proposals -// in order and stop/removes the chain if the stop time has passed, -// otherwise it will break out of loop and return. Executed proposals are deleted. -// -// See: https://github.com/cosmos/ibc/blob/main/spec/app/ics-028-cross-chain-validation/methods.md#ccv-pcf-bblock-ccr1 -// Spec tag: [CCV-PCF-BBLOCK-CCR.1] +// 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) { - propsToExecute := k.GetConsumerRemovalPropsToExecute(ctx) - - for _, prop := range propsToExecute { + for _, consumerId := range k.GetLaunchedConsumersReadyToStop(ctx) { // stop consumer chain in a cached context to handle errors - cachedCtx, writeFn, err := k.StopConsumerChainInCachedCtx(ctx, prop) + cachedCtx, writeFn := ctx.CacheContext() + + stopTime, found := k.GetConsumerIdToStopTime(ctx, consumerId) + if !found { + ctx.Logger().Info("this chain (%s) is not meant to be stopped", consumerId) + continue + } + + err := k.StopConsumerChain(cachedCtx, consumerId, true) if err != nil { - // drop the proposal ctx.Logger().Info("consumer chain could not be stopped: %w", err) continue } // The cached context is created with a new EventManager so we merge the event // into the original context + // TODO (PERMISSIONLESS): verify this here and in the initialized chains to launch ctx.EventManager().EmitEvents(cachedCtx.EventManager().Events()) - // write cache + + k.SetConsumerIdToPhase(cachedCtx, consumerId, Stopped) writeFn() - k.Logger(ctx).Info("executed consumer removal proposal", - "consumer id", prop.ConsumerId, - "title", prop.Title, - "stop time", prop.StopTime.UTC(), + k.Logger(ctx).Info("executed consumer removal", + "consumer id", consumerId, + "stop time", stopTime, ) } - // delete the executed proposals - k.DeletePendingConsumerRemovalProps(ctx, propsToExecute...) } -// GetConsumerRemovalPropsToExecute iterates over the pending consumer removal proposals -// and returns an ordered list of consumer removal proposals to be executed, -// ie. consumer chains to be stopped and removed from the provider chain. -// A prop is included in the returned list if its proposed stop time has passed. +//// GetConsumerRemovalPropsToExecute iterates over the pending consumer removal proposals +//// and returns an ordered list of consumer removal proposals to be executed, +//// ie. consumer chains to be stopped and removed from the provider chain. +//// A prop is included in the returned list if its proposed stop time has passed. +//// +//// Note: this method is split out from BeginBlockCCR to be easily unit tested. +//func (k Keeper) GetConsumerRemovalPropsToExecute(ctx sdk.Context) []types.ConsumerRemovalProposal { +// // store the (to be) executed consumer removal proposals in order +// propsToExecute := []types.ConsumerRemovalProposal{} // -// Note: this method is split out from BeginBlockCCR to be easily unit tested. -func (k Keeper) GetConsumerRemovalPropsToExecute(ctx sdk.Context) []types.ConsumerRemovalProposal { - // store the (to be) executed consumer removal proposals in order - propsToExecute := []types.ConsumerRemovalProposal{} - - store := ctx.KVStore(k.storeKey) - iterator := storetypes.KVStorePrefixIterator(store, types.PendingCRPKeyPrefix()) - defer iterator.Close() - - for ; iterator.Valid(); iterator.Next() { - var prop types.ConsumerRemovalProposal - err := prop.Unmarshal(iterator.Value()) - if err != nil { - // An error here would indicate something is very wrong, - // the ConsumerRemovalProposal is assumed to be correctly serialized in SetPendingConsumerRemovalProp. - panic(fmt.Errorf("failed to unmarshal consumer removal proposal: %w", err)) - } - - // If current block time is equal to or after stop time, proposal is ready to be executed - if !ctx.BlockTime().Before(prop.StopTime) { - propsToExecute = append(propsToExecute, prop) - } else { - // No more proposals to check, since they're stored/ordered by timestamp. - break - } - } - - return propsToExecute -} +// store := ctx.KVStore(k.storeKey) +// iterator := storetypes.KVStorePrefixIterator(store, types.PendingCRPKeyPrefix()) +// defer iterator.Close() +// +// for ; iterator.Valid(); iterator.Next() { +// var prop types.ConsumerRemovalProposal +// err := prop.Unmarshal(iterator.Value()) +// if err != nil { +// // An error here would indicate something is very wrong, +// // the ConsumerRemovalProposal is assumed to be correctly serialized in SetPendingConsumerRemovalProp. +// panic(fmt.Errorf("failed to unmarshal consumer removal proposal: %w", err)) +// } +// +// // If current block time is equal to or after stop time, proposal is ready to be executed +// if !ctx.BlockTime().Before(prop.StopTime) { +// propsToExecute = append(propsToExecute, prop) +// } else { +// // No more proposals to check, since they're stored/ordered by timestamp. +// break +// } +// } +// +// return propsToExecute +//} // GetAllPendingConsumerRemovalProps iterates through the pending consumer removal proposals. // diff --git a/x/ccv/provider/keeper/proposal_test.go b/x/ccv/provider/keeper/proposal_test.go index 92c2c7d1d3..e58c582517 100644 --- a/x/ccv/provider/keeper/proposal_test.go +++ b/x/ccv/provider/keeper/proposal_test.go @@ -333,120 +333,80 @@ func TestStopConsumerChain(t *testing.T) { } } -// TestPendingConsumerRemovalPropDeletion tests the getting/setting -// and deletion methods for pending consumer removal props -func TestPendingConsumerRemovalPropDeletion(t *testing.T) { - testCases := []struct { - providertypes.ConsumerRemovalProposal - ExpDeleted bool - }{ - { - ConsumerRemovalProposal: providertypes.ConsumerRemovalProposal{ConsumerId: "8", StopTime: time.Now().UTC()}, - ExpDeleted: true, - }, - { - ConsumerRemovalProposal: providertypes.ConsumerRemovalProposal{ConsumerId: "9", StopTime: time.Now().UTC().Add(time.Hour)}, - ExpDeleted: false, - }, - } - providerKeeper, ctx, ctrl, _ := testkeeper.GetProviderKeeperAndCtx(t, testkeeper.NewInMemKeeperParams(t)) - defer ctrl.Finish() - - for _, tc := range testCases { - tc := tc - providerKeeper.SetPendingConsumerRemovalProp(ctx, &tc.ConsumerRemovalProposal) - } - - ctx = ctx.WithBlockTime(time.Now().UTC()) - - propsToExecute := providerKeeper.GetConsumerRemovalPropsToExecute(ctx) - // Delete consumer removal proposals, same as what would be done by BeginBlockCCR - providerKeeper.DeletePendingConsumerRemovalProps(ctx, propsToExecute...) - numDeleted := 0 - for _, tc := range testCases { - res := providerKeeper.PendingConsumerRemovalPropExists(ctx, tc.ConsumerId, tc.StopTime) - if !tc.ExpDeleted { - require.NotEmpty(t, res, "consumer removal prop was deleted: %s %s", tc.ConsumerId, tc.StopTime.String()) - continue - } - require.Empty(t, res, "consumer removal prop was not deleted %s %s", tc.ConsumerId, tc.StopTime.String()) - require.Equal(t, propsToExecute[numDeleted].ConsumerId, tc.ConsumerId) - numDeleted += 1 - } -} - +// 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) - } -} +//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) +// } +//} // Test getting both matured and pending consumer removal proposals func TestGetAllConsumerRemovalProps(t *testing.T) { @@ -894,10 +854,6 @@ func TestBeginBlockInit(t *testing.T) { require.False(t, found) } -// TestBeginBlockCCR tests BeginBlockCCR against the spec. -// -// See: https://github.com/cosmos/ibc/blob/main/spec/app/ics-028-cross-chain-validation/methods.md#ccv-pcf-bblock-ccr1 -// Spec tag: [CCV-PCF-BBLOCK-CCR.1] func TestBeginBlockCCR(t *testing.T) { now := time.Now().UTC() @@ -907,28 +863,23 @@ func TestBeginBlockCCR(t *testing.T) { defer ctrl.Finish() ctx = ctx.WithBlockTime(now) - pendingProps := []*providertypes.ConsumerRemovalProposal{ - providertypes.NewConsumerRemovalProposal( - "title", "description", "chain1", now.Add(-time.Hour).UTC(), - ).(*providertypes.ConsumerRemovalProposal), - providertypes.NewConsumerRemovalProposal( - "title", "description", "chain2", now, - ).(*providertypes.ConsumerRemovalProposal), - providertypes.NewConsumerRemovalProposal( - "title", "description", "chain3", now.Add(time.Hour).UTC(), - ).(*providertypes.ConsumerRemovalProposal), - } + chainIds := []string{"chain1", "chain2", "chain3"} + consumerIds := []string{"consumerId1", "consumerId2", "consumerId3"} + providerKeeper.SetConsumerIdToStopTime(ctx, consumerIds[0], now.Add(-time.Hour)) + providerKeeper.SetConsumerIdToStopTime(ctx, consumerIds[1], now) + providerKeeper.SetConsumerIdToStopTime(ctx, consumerIds[2], now.Add(time.Hour)) // // Mock expectations // expectations := []*gomock.Call{} - for _, prop := range pendingProps { - // A consumer chain is setup corresponding to each prop, making these mocks necessary + 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{}, []int64{}, 1) expectations = append(expectations, testkeeper.GetMocksForCreateConsumerClient(ctx, &mocks, - prop.ConsumerId, clienttypes.NewHeight(2, 3))...) - expectations = append(expectations, testkeeper.GetMocksForSetConsumerChain(ctx, &mocks, prop.ConsumerId)...) + 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)...) @@ -939,34 +890,28 @@ func TestBeginBlockCCR(t *testing.T) { // // Remaining setup // - for _, prop := range pendingProps { - // Setup a valid consumer chain for each prop + for i, consumerId := range consumerIds { + // Setup a valid consumer chain for each consumerId initializationRecord := testkeeper.GetTestInitializationRecord() initializationRecord.InitialHeight = clienttypes.NewHeight(2, 3) registrationRecord := testkeeper.GetTestRegistrationRecord() - registrationRecord.ChainId = prop.ConsumerId + registrationRecord.ChainId = chainIds[i] - providerKeeper.SetConsumerIdToRegistrationRecord(ctx, prop.ConsumerId, registrationRecord) - providerKeeper.SetConsumerIdToInitializationRecord(ctx, prop.ConsumerId, initializationRecord) - providerKeeper.SetConsumerIdToUpdateRecord(ctx, prop.ConsumerId, testkeeper.GetTestUpdateRecord()) - providerKeeper.SetConsumerIdToPhase(ctx, prop.ConsumerId, providerkeeper.Initialized) - providerKeeper.SetClientIdToConsumerId(ctx, "clientID", prop.ConsumerId) + providerKeeper.SetConsumerIdToRegistrationRecord(ctx, consumerId, registrationRecord) + providerKeeper.SetConsumerIdToInitializationRecord(ctx, consumerId, initializationRecord) + providerKeeper.SetConsumerIdToUpdateRecord(ctx, consumerId, testkeeper.GetTestUpdateRecord()) + providerKeeper.SetConsumerIdToPhase(ctx, consumerId, providerkeeper.Initialized) + providerKeeper.SetClientIdToConsumerId(ctx, "clientID", consumerId) - err := providerKeeper.CreateConsumerClient(ctx, prop.ConsumerId) + err := providerKeeper.CreateConsumerClient(ctx, consumerId) require.NoError(t, err) err = providerKeeper.SetConsumerChain(ctx, "channelID") require.NoError(t, err) - // Set removal props for all consumer chains - providerKeeper.SetPendingConsumerRemovalProp(ctx, prop) + // after we have created the consumer client, the chain is considered launched and hence we could later stop the chain + providerKeeper.SetConsumerIdToPhase(ctx, consumerId, providerkeeper.Launched) } - // Add an invalid prop to the store with an non-existing chain id - invalidProp := providertypes.NewConsumerRemovalProposal( - "title", "description", "chain4", now.Add(-time.Hour).UTC(), - ).(*providertypes.ConsumerRemovalProposal) - providerKeeper.SetPendingConsumerRemovalProp(ctx, invalidProp) - // // Test execution // @@ -974,16 +919,14 @@ func TestBeginBlockCCR(t *testing.T) { providerKeeper.BeginBlockCCR(ctx) // Only the 3rd (final) proposal is still stored as pending - found := providerKeeper.PendingConsumerRemovalPropExists( - ctx, pendingProps[0].ConsumerId, pendingProps[0].StopTime) - require.False(t, found) - found = providerKeeper.PendingConsumerRemovalPropExists( - ctx, pendingProps[1].ConsumerId, pendingProps[1].StopTime) - require.False(t, found) - found = providerKeeper.PendingConsumerRemovalPropExists( - ctx, pendingProps[2].ConsumerId, pendingProps[2].StopTime) + phase, found := providerKeeper.GetConsumerIdToPhase(ctx, consumerIds[0]) require.True(t, found) - found = providerKeeper.PendingConsumerRemovalPropExists( - ctx, invalidProp.ConsumerId, invalidProp.StopTime) - require.False(t, found) + require.Equal(t, providerkeeper.Stopped, phase) + phase, found = providerKeeper.GetConsumerIdToPhase(ctx, consumerIds[1]) + require.True(t, found) + require.Equal(t, providerkeeper.Stopped, phase) + // third chain had a stopTime in the future and hence did not stop + phase, found = providerKeeper.GetConsumerIdToPhase(ctx, consumerIds[2]) + require.True(t, found) + require.Equal(t, providerkeeper.Launched, phase) } diff --git a/x/ccv/provider/module.go b/x/ccv/provider/module.go index c8e646ded9..be7e5e7c4a 100644 --- a/x/ccv/provider/module.go +++ b/x/ccv/provider/module.go @@ -167,10 +167,11 @@ func (AppModule) ConsensusVersion() uint64 { return 8 } // BeginBlock implements the AppModule interface func (am AppModule) BeginBlock(ctx context.Context) error { - sdkCtx := sdk.UnwrapSDKContext(ctx) // Create clients to consumer chains that are due to be spawned via pending consumer addition proposals + sdkCtx := sdk.UnwrapSDKContext(ctx) + // Create clients to consumer chains that are due to be spawned am.keeper.BeginBlockInit(sdkCtx) - // Stop and remove state for any consumer chains that are due to be stopped via pending consumer removal proposals + // Stop and remove state for any consumer chains that are due to be stopped am.keeper.BeginBlockCCR(sdkCtx) // Check for replenishing slash meter before any slash packets are processed for this block am.keeper.BeginBlockCIS(sdkCtx) diff --git a/x/ccv/provider/proposal_handler.go b/x/ccv/provider/proposal_handler.go index daae9699a6..01d92d3e9e 100644 --- a/x/ccv/provider/proposal_handler.go +++ b/x/ccv/provider/proposal_handler.go @@ -20,7 +20,7 @@ func NewProviderProposalHandler(k keeper.Keeper) govv1beta1.Handler { case *types.ConsumerAdditionProposal: return nil case *types.ConsumerRemovalProposal: - return k.HandleLegacyConsumerRemovalProposal(ctx, c) + return nil case *types.ChangeRewardDenomsProposal: return k.HandleLegacyConsumerRewardDenomProposal(ctx, c) default: diff --git a/x/ccv/provider/types/keys.go b/x/ccv/provider/types/keys.go index 8b4bc51800..c84d9e24ba 100644 --- a/x/ccv/provider/types/keys.go +++ b/x/ccv/provider/types/keys.go @@ -135,6 +135,8 @@ const ( ConsumerIdToPhaseKeyName = "ConsumerIdToPhaseKey" + ConsumerIdToStopTimeKeyName = "ConsumerIdToStopTimeKey" + ClientIdToConsumerIdKeyName = "ClientIdToConsumerIdKey" ) @@ -334,8 +336,11 @@ func getKeyPrefixes() map[string]byte { // ConsumerIdToPhaseKeyName is the key for storing the phase of a consumer chain with the given consumer id ConsumerIdToPhaseKeyName: 47, + // ConsumerIdToStopTimeKeyName is the key for storing the stop time of a consumer chain that is to be removed + ConsumerIdToStopTimeKeyName: 48, + // ClientIdToConsumerIdKeyName is the key for storing the consumer id for the given client id - ClientIdToConsumerIdKeyName: 48, + ClientIdToConsumerIdKeyName: 49, // NOTE: DO NOT ADD NEW BYTE PREFIXES HERE WITHOUT ADDING THEM TO TestPreserveBytePrefix() IN keys_test.go } @@ -742,6 +747,17 @@ func ConsumerIdToPhaseKey(consumerId string) []byte { return ConsumerIdWithLenKey(mustGetKeyPrefix(ConsumerIdToPhaseKeyName), consumerId) } +// ConsumerIdToStopTimeKey returns the key used to store the stop time that corresponds to a to-be-stopped chain with consumer id +func ConsumerIdToStopTimeKey(consumerId string) []byte { + return ConsumerIdWithLenKey(mustGetKeyPrefix(ConsumerIdToStopTimeKeyName), consumerId) +} + +// ConsumerIdToStopTimeKeyNamePrefix returns the key prefix for storing the stop times of consumer chains +// that are about to be stopped +func ConsumerIdToStopTimeKeyNamePrefix() []byte { + return []byte{mustGetKeyPrefix(ConsumerIdToStopTimeKeyName)} +} + // ClientIdToConsumerIdKey returns the consumer id that corresponds to this client id func ClientIdToConsumerIdKey(clientId string) []byte { clientIdLength := len(clientId) diff --git a/x/ccv/provider/types/keys_test.go b/x/ccv/provider/types/keys_test.go index c3555ab376..b7de3cc0d2 100644 --- a/x/ccv/provider/types/keys_test.go +++ b/x/ccv/provider/types/keys_test.go @@ -126,7 +126,9 @@ func TestPreserveBytePrefix(t *testing.T) { i++ require.Equal(t, uint8(47), providertypes.ConsumerIdToPhaseKey("consumerId")[0]) i++ - require.Equal(t, uint8(48), providertypes.ClientIdToConsumerIdKey("clientId")[0]) + require.Equal(t, uint8(48), providertypes.ConsumerIdToStopTimeKey("consumerId")[0]) + i++ + require.Equal(t, uint8(49), providertypes.ClientIdToConsumerIdKey("clientId")[0]) i++ prefixes := providertypes.GetAllKeyPrefixes() @@ -198,6 +200,7 @@ func getAllFullyDefinedKeys() [][]byte { providertypes.ConsumerIdToUpdateRecordKey("consumerId"), providertypes.ConsumerIdToOwnerAddressKey("consumerId"), providertypes.ConsumerIdToPhaseKey("consumerId"), + providertypes.ConsumerIdToStopTimeKey("consumerId"), providertypes.ClientIdToConsumerIdKey("clientId"), } } diff --git a/x/ccv/provider/types/msg.go b/x/ccv/provider/types/msg.go index 68861d86c6..68479ab899 100644 --- a/x/ccv/provider/types/msg.go +++ b/x/ccv/provider/types/msg.go @@ -51,6 +51,7 @@ var ( _ sdk.HasValidateBasic = (*MsgChangeRewardDenoms)(nil) _ sdk.HasValidateBasic = (*MsgSubmitConsumerMisbehaviour)(nil) _ sdk.HasValidateBasic = (*MsgSubmitConsumerDoubleVoting)(nil) + // TODO (PERMISSIONLESS) add extensive checks, etc. _ sdk.HasValidateBasic = (*MsgRegisterConsumer)(nil) _ sdk.HasValidateBasic = (*MsgInitializeConsumer)(nil) _ sdk.HasValidateBasic = (*MsgUpdateConsumer)(nil)