From 300530f18130d3f3f0e18d51e6b07ff4cc547406 Mon Sep 17 00:00:00 2001 From: mpoke Date: Thu, 5 Sep 2024 18:29:18 +0200 Subject: [PATCH] replace CanLaunch with InitializeConsumer --- x/ccv/provider/keeper/consumer_lifecycle.go | 26 ++-- .../keeper/consumer_lifecycle_test.go | 112 ++++++++++++------ x/ccv/provider/keeper/msg_server.go | 10 +- x/ccv/provider/types/errors.go | 1 - x/ccv/provider/types/msg.go | 4 - x/ccv/provider/types/msg_test.go | 2 +- 6 files changed, 92 insertions(+), 63 deletions(-) diff --git a/x/ccv/provider/keeper/consumer_lifecycle.go b/x/ccv/provider/keeper/consumer_lifecycle.go index 7c1817abd0..20014c3c35 100644 --- a/x/ccv/provider/keeper/consumer_lifecycle.go +++ b/x/ccv/provider/keeper/consumer_lifecycle.go @@ -36,13 +36,12 @@ func (k Keeper) PrepareConsumerForLaunch(ctx sdk.Context, consumerId string, pre 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 +// InitializeConsumer tries to move a consumer with `consumerId` to the initialized phase. +// If successfull, it returns the spawn time and true. +func (k Keeper) InitializeConsumer(ctx sdk.Context, consumerId string) (time.Time, bool) { + // a chain needs to be in the registered or initialized phase phase := k.GetConsumerPhase(ctx, consumerId) - if phase == types.CONSUMER_PHASE_LAUNCHED || phase == types.CONSUMER_PHASE_STOPPED { + if phase != types.CONSUMER_PHASE_REGISTERED && phase != types.CONSUMER_PHASE_INITIALIZED { return time.Time{}, false } @@ -51,17 +50,14 @@ func (k Keeper) CanLaunch(ctx sdk.Context, consumerId string) (time.Time, bool) return time.Time{}, false } - spawnTimeIsNotZero := !initializationParameters.SpawnTime.Equal(time.Time{}) - - genesisHashSet := initializationParameters.GenesisHash != nil - binaryHashSet := initializationParameters.BinaryHash != nil + // the spawn time needs to be positive + if initializationParameters.SpawnTime.IsZero() { + return time.Time{}, false + } - consumerRedistributionFractionSet := initializationParameters.ConsumerRedistributionFraction != "" - blocksPerDistributionTransmissionSet := initializationParameters.BlocksPerDistributionTransmission > 0 - historicalEntriesSet := initializationParameters.HistoricalEntries > 0 + k.SetConsumerPhase(ctx, consumerId, types.CONSUMER_PHASE_INITIALIZED) - return initializationParameters.SpawnTime, spawnTimeIsNotZero && genesisHashSet && binaryHashSet && consumerRedistributionFractionSet && - blocksPerDistributionTransmissionSet && historicalEntriesSet + return initializationParameters.SpawnTime, true } // BeginBlockLaunchConsumers launches initialized consumers that are ready to launch diff --git a/x/ccv/provider/keeper/consumer_lifecycle_test.go b/x/ccv/provider/keeper/consumer_lifecycle_test.go index 3d59fec53d..76d654c0c3 100644 --- a/x/ccv/provider/keeper/consumer_lifecycle_test.go +++ b/x/ccv/provider/keeper/consumer_lifecycle_test.go @@ -52,45 +52,85 @@ func TestPrepareConsumerForLaunch(t *testing.T) { 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.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.CONSUMER_PHASE_LAUNCHED) - _, canLaunch = providerKeeper.CanLaunch(ctx, "consumerId") - require.False(t, canLaunch) +func TestInitializeConsumer(t *testing.T) { + now := time.Now().UTC() + consumerId := "13" + + testCases := []struct { + name string + spawnTime time.Time + setup func(*providerkeeper.Keeper, sdk.Context, time.Time) + expInitialized bool + }{ + { + name: "valid", + spawnTime: now, + setup: func(pk *providerkeeper.Keeper, ctx sdk.Context, spawnTime time.Time) { + pk.SetConsumerPhase(ctx, consumerId, providertypes.CONSUMER_PHASE_REGISTERED) + err := pk.SetConsumerInitializationParameters(ctx, consumerId, + providertypes.ConsumerInitializationParameters{ + SpawnTime: spawnTime, + }) + require.NoError(t, err) + }, + expInitialized: true, + }, + { + name: "invalid: no phase", + spawnTime: now, + setup: func(pk *providerkeeper.Keeper, ctx sdk.Context, spawnTime time.Time) { + }, + expInitialized: false, + }, + { + name: "invalid: wrong phase", + spawnTime: now, + setup: func(pk *providerkeeper.Keeper, ctx sdk.Context, spawnTime time.Time) { + pk.SetConsumerPhase(ctx, consumerId, providertypes.CONSUMER_PHASE_LAUNCHED) + err := pk.SetConsumerInitializationParameters(ctx, consumerId, + providertypes.ConsumerInitializationParameters{ + SpawnTime: spawnTime, + }) + require.NoError(t, err) + }, + expInitialized: false, + }, + { + name: "invalid: no init params", + spawnTime: now, + setup: func(pk *providerkeeper.Keeper, ctx sdk.Context, spawnTime time.Time) { + pk.SetConsumerPhase(ctx, consumerId, providertypes.CONSUMER_PHASE_REGISTERED) + }, + expInitialized: false, + }, + { + name: "invalid: zero spawn time", + spawnTime: now, + setup: func(pk *providerkeeper.Keeper, ctx sdk.Context, spawnTime time.Time) { + pk.SetConsumerPhase(ctx, consumerId, providertypes.CONSUMER_PHASE_REGISTERED) + err := pk.SetConsumerInitializationParameters(ctx, consumerId, + providertypes.ConsumerInitializationParameters{ + SpawnTime: time.Time{}, + }) + require.NoError(t, err) + }, + expInitialized: false, + }, + } - // cannot launch a stopped chain - providerKeeper.SetConsumerPhase(ctx, "consumerId", providertypes.CONSUMER_PHASE_STOPPED) - _, canLaunch = providerKeeper.CanLaunch(ctx, "consumerId") - require.False(t, canLaunch) + for _, tc := range testCases { + pk, ctx, ctrl, _ := testkeeper.GetProviderKeeperAndCtx(t, testkeeper.NewInMemKeeperParams(t)) + defer ctrl.Finish() - // initialized chain can launch - providerKeeper.SetConsumerPhase(ctx, "consumerId", providertypes.CONSUMER_PHASE_INITIALIZED) - _, canLaunch = providerKeeper.CanLaunch(ctx, "consumerId") - require.True(t, canLaunch) + tc.setup(&pk, ctx, tc.spawnTime) - // 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) + spawnTime, initialized := pk.InitializeConsumer(ctx, consumerId) + require.Equal(t, tc.expInitialized, initialized, tc.name) + if initialized { + require.Equal(t, tc.spawnTime, spawnTime, tc.name) + require.Equal(t, providertypes.CONSUMER_PHASE_INITIALIZED, pk.GetConsumerPhase(ctx, consumerId)) + } + } } // TestBeginBlockInit directly tests BeginBlockLaunchConsumers against the spec using helpers defined above. diff --git a/x/ccv/provider/keeper/msg_server.go b/x/ccv/provider/keeper/msg_server.go index 9414ffc93e..e5148b5a37 100644 --- a/x/ccv/provider/keeper/msg_server.go +++ b/x/ccv/provider/keeper/msg_server.go @@ -405,10 +405,9 @@ func (k msgServer) CreateConsumer(goCtx context.Context, msg *types.MsgCreateCon "cannot set power shaping parameters") } - if spawnTime, canLaunch := k.Keeper.CanLaunch(ctx, consumerId); canLaunch { - k.Keeper.SetConsumerPhase(ctx, consumerId, types.CONSUMER_PHASE_INITIALIZED) + if spawnTime, initialized := k.Keeper.InitializeConsumer(ctx, consumerId); initialized { if err := k.Keeper.PrepareConsumerForLaunch(ctx, consumerId, time.Time{}, spawnTime); err != nil { - return &resp, errorsmod.Wrapf(types.ErrCannotPrepareForLaunch, + return &resp, errorsmod.Wrapf(ccvtypes.ErrInvalidConsumerState, "cannot prepare chain with consumer id (%s) for launch", consumerId) } } @@ -562,10 +561,9 @@ func (k msgServer) UpdateConsumer(goCtx context.Context, msg *types.MsgUpdateCon "a move to a new owner address that is not the gov module can only be done if `Top N` is set to 0") } - if spawnTime, canLaunch := k.Keeper.CanLaunch(ctx, consumerId); canLaunch { - k.Keeper.SetConsumerPhase(ctx, consumerId, types.CONSUMER_PHASE_INITIALIZED) + if spawnTime, initialized := k.Keeper.InitializeConsumer(ctx, consumerId); initialized { if err := k.Keeper.PrepareConsumerForLaunch(ctx, consumerId, previousSpawnTime, spawnTime); err != nil { - return &resp, errorsmod.Wrapf(types.ErrCannotPrepareForLaunch, + return &resp, errorsmod.Wrapf(ccvtypes.ErrInvalidConsumerState, "cannot prepare chain with consumer id (%s) for launch", consumerId) } } diff --git a/x/ccv/provider/types/errors.go b/x/ccv/provider/types/errors.go index 933e363f73..2d87ee1345 100644 --- a/x/ccv/provider/types/errors.go +++ b/x/ccv/provider/types/errors.go @@ -35,7 +35,6 @@ var ( ErrInvalidTransformToTopN = errorsmod.Register(ModuleName, 39, "invalid transform to Top N chain") ErrInvalidTransformToOptIn = errorsmod.Register(ModuleName, 40, "invalid transform to Opt In chain") ErrCannotCreateTopNChain = errorsmod.Register(ModuleName, 41, "cannot create Top N chain outside permissionlessly") - ErrCannotPrepareForLaunch = errorsmod.Register(ModuleName, 42, "cannot prepare chain for launch") ErrInvalidRemovalTime = errorsmod.Register(ModuleName, 43, "invalid removal time") ErrInvalidMsgCreateConsumer = errorsmod.Register(ModuleName, 44, "invalid create consumer message") ErrInvalidMsgUpdateConsumer = errorsmod.Register(ModuleName, 45, "invalid update consumer message") diff --git a/x/ccv/provider/types/msg.go b/x/ccv/provider/types/msg.go index 66517ca773..f7ddd1ad4a 100644 --- a/x/ccv/provider/types/msg.go +++ b/x/ccv/provider/types/msg.go @@ -533,10 +533,6 @@ func ValidateInitializationParameters(initializationParameters ConsumerInitializ return errorsmod.Wrapf(ErrInvalidConsumerInitializationParameters, "BinaryHash: %s", err.Error()) } - if initializationParameters.SpawnTime.IsZero() { - return errorsmod.Wrap(ErrInvalidConsumerInitializationParameters, "SpawnTime cannot be zero") - } - if err := ccvtypes.ValidateStringFraction(initializationParameters.ConsumerRedistributionFraction); err != nil { return errorsmod.Wrapf(ErrInvalidConsumerInitializationParameters, "ConsumerRedistributionFraction: %s", err.Error()) } diff --git a/x/ccv/provider/types/msg_test.go b/x/ccv/provider/types/msg_test.go index 72e4712f59..c05a628ac4 100644 --- a/x/ccv/provider/types/msg_test.go +++ b/x/ccv/provider/types/msg_test.go @@ -159,7 +159,7 @@ func TestValidateInitializationParameters(t *testing.T) { HistoricalEntries: 10000, DistributionTransmissionChannel: "", }, - valid: false, + valid: true, }, { name: "invalid - zero duration",