From 29d790c2e97317a5e80a490db537de2a7fa79d4d Mon Sep 17 00:00:00 2001 From: insumity Date: Mon, 26 Aug 2024 15:13:47 +0200 Subject: [PATCH 1/5] fixed bug on removing previous spawn time & added tests --- testutil/ibc_testing/generic_setup.go | 2 +- testutil/keeper/unit_test_helpers.go | 2 +- x/ccv/provider/keeper/keeper.go | 2 +- x/ccv/provider/keeper/msg_server.go | 57 ++- x/ccv/provider/keeper/msg_server_test.go | 112 ++++++ x/ccv/provider/keeper/permissionless.go | 240 +++++------- x/ccv/provider/keeper/permissionless_test.go | 372 +++++++++++++++---- x/ccv/provider/keeper/proposal.go | 8 +- x/ccv/provider/keeper/proposal_test.go | 8 +- 9 files changed, 535 insertions(+), 268 deletions(-) diff --git a/testutil/ibc_testing/generic_setup.go b/testutil/ibc_testing/generic_setup.go index d9aa02d732..12fe71a34e 100644 --- a/testutil/ibc_testing/generic_setup.go +++ b/testutil/ibc_testing/generic_setup.go @@ -159,7 +159,7 @@ func AddConsumer[Tp testutil.ProviderApp, Tc testutil.ConsumerApp]( providerKeeper.SetConsumerInitializationParameters(providerChain.GetContext(), consumerId, initializationParameters) providerKeeper.SetConsumerPowerShapingParameters(providerChain.GetContext(), consumerId, powerShapingParameters) providerKeeper.SetConsumerPhase(providerChain.GetContext(), consumerId, keeper.Initialized) - providerKeeper.AppendSpawnTimeForConsumerToBeLaunched(providerChain.GetContext(), consumerId, coordinator.CurrentTime) + providerKeeper.AppendConsumerToBeLaunchedOnSpawnTime(providerChain.GetContext(), consumerId, coordinator.CurrentTime) // opt-in all validators lastVals, err := providerApp.GetProviderKeeper().GetLastBondedValidators(providerChain.GetContext()) diff --git a/testutil/keeper/unit_test_helpers.go b/testutil/keeper/unit_test_helpers.go index ef8d3a747f..b27129c543 100644 --- a/testutil/keeper/unit_test_helpers.go +++ b/testutil/keeper/unit_test_helpers.go @@ -283,7 +283,7 @@ func GetTestConsumerMetadata() providertypes.ConsumerMetadata { func GetTestInitializationParameters() providertypes.ConsumerInitializationParameters { initialHeight := clienttypes.NewHeight(4, 5) - spawnTime := time.Now() + spawnTime := time.Now().UTC() ccvTimeoutPeriod := types.DefaultCCVTimeoutPeriod transferTimeoutPeriod := types.DefaultTransferTimeoutPeriod unbondingPeriod := types.DefaultConsumerUnbondingPeriod diff --git a/x/ccv/provider/keeper/keeper.go b/x/ccv/provider/keeper/keeper.go index 46f641f660..66df230b09 100644 --- a/x/ccv/provider/keeper/keeper.go +++ b/x/ccv/provider/keeper/keeper.go @@ -750,7 +750,7 @@ func (k Keeper) GetAllActiveConsumerIds(ctx sdk.Context) []string { } consumerIds := []string{} - for i := uint64(0); i <= latestConsumerId; i++ { + for i := uint64(0); i < latestConsumerId; i++ { consumerId := fmt.Sprintf("%d", i) phase, foundPhase := k.GetConsumerPhase(ctx, consumerId) if !foundPhase || (phase != Registered && phase != Initialized && phase != Launched) { diff --git a/x/ccv/provider/keeper/msg_server.go b/x/ccv/provider/keeper/msg_server.go index 9550b7d620..eb9f9b4f75 100644 --- a/x/ccv/provider/keeper/msg_server.go +++ b/x/ccv/provider/keeper/msg_server.go @@ -106,11 +106,11 @@ func (k msgServer) RemoveConsumer( previousStopTime, err := k.Keeper.GetConsumerStopTime(ctx, consumerId) if err == nil { - k.Keeper.RemoveConsumerFromToBeStoppedConsumers(ctx, consumerId, previousStopTime) + k.Keeper.RemoveConsumerToBeStoppedFromStopTime(ctx, consumerId, previousStopTime) } k.Keeper.SetConsumerStopTime(ctx, consumerId, msg.StopTime) - k.Keeper.AppendStopTimeForConsumerToBeStopped(ctx, consumerId, msg.StopTime) + k.Keeper.AppendConsumerToBeStoppedOnStopTime(ctx, consumerId, msg.StopTime) return &types.MsgRemoveConsumerResponse{}, nil } @@ -309,20 +309,20 @@ func (k msgServer) SetConsumerCommissionRate(goCtx context.Context, msg *types.M func (k msgServer) CreateConsumer(goCtx context.Context, msg *types.MsgCreateConsumer) (*types.MsgCreateConsumerResponse, error) { ctx := sdk.UnwrapSDKContext(goCtx) - consumerId := k.FetchAndIncrementConsumerId(ctx) + consumerId := k.Keeper.FetchAndIncrementConsumerId(ctx) - k.SetConsumerOwnerAddress(ctx, consumerId, msg.Signer) - k.SetConsumerChainId(ctx, consumerId, msg.ChainId) - k.SetConsumerPhase(ctx, consumerId, Registered) + k.Keeper.SetConsumerOwnerAddress(ctx, consumerId, msg.Signer) + k.Keeper.SetConsumerChainId(ctx, consumerId, msg.ChainId) + k.Keeper.SetConsumerPhase(ctx, consumerId, Registered) - if err := k.SetConsumerMetadata(ctx, consumerId, msg.Metadata); err != nil { + if err := k.Keeper.SetConsumerMetadata(ctx, consumerId, msg.Metadata); err != nil { return &types.MsgCreateConsumerResponse{}, errorsmod.Wrapf(types.ErrInvalidConsumerMetadata, "cannot set consumer metadata: %s", err.Error()) } // initialization parameters are optional and hence could be nil if msg.InitializationParameters != nil { - if err := k.SetConsumerInitializationParameters(ctx, consumerId, *msg.InitializationParameters); err != nil { + if err := k.Keeper.SetConsumerInitializationParameters(ctx, consumerId, *msg.InitializationParameters); err != nil { return &types.MsgCreateConsumerResponse{}, errorsmod.Wrapf(types.ErrInvalidConsumerInitializationParameters, "cannot set consumer initialization parameters: %s", err.Error()) } @@ -334,15 +334,15 @@ func (k msgServer) CreateConsumer(goCtx context.Context, msg *types.MsgCreateCon return &types.MsgCreateConsumerResponse{}, errorsmod.Wrap(types.ErrCannotCreateTopNChain, "cannot create a Top N chain using the `MsgCreateConsumer` message; use `MsgUpdateConsumer` instead") } - if err := k.SetConsumerPowerShapingParameters(ctx, consumerId, *msg.PowerShapingParameters); err != nil { + if err := k.Keeper.SetConsumerPowerShapingParameters(ctx, consumerId, *msg.PowerShapingParameters); err != nil { return &types.MsgCreateConsumerResponse{}, errorsmod.Wrapf(types.ErrInvalidPowerShapingParameters, "cannot set power shaping parameters") } } - if spawnTime, canLaunch := k.CanLaunch(ctx, consumerId); canLaunch { - k.SetConsumerPhase(ctx, consumerId, Initialized) - k.PrepareConsumerForLaunch(ctx, consumerId, time.Time{}, spawnTime) + if spawnTime, canLaunch := k.Keeper.CanLaunch(ctx, consumerId); canLaunch { + k.Keeper.SetConsumerPhase(ctx, consumerId, Initialized) + k.Keeper.PrepareConsumerForLaunch(ctx, consumerId, time.Time{}, spawnTime) } return &types.MsgCreateConsumerResponse{ConsumerId: consumerId}, nil @@ -353,20 +353,12 @@ func (k msgServer) UpdateConsumer(goCtx context.Context, msg *types.MsgUpdateCon ctx := sdk.UnwrapSDKContext(goCtx) consumerId := msg.ConsumerId - phase, found := k.GetConsumerPhase(ctx, consumerId) + phase, found := k.Keeper.GetConsumerPhase(ctx, consumerId) if found && phase == Stopped { return &types.MsgUpdateConsumerResponse{}, errorsmod.Wrapf(types.ErrInvalidPhase, "cannot update consumer chain that is in the stopped phase: %s", consumerId) } - // 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) != "" { - if _, err := k.accountKeeper.AddressCodec().StringToBytes(msg.NewOwnerAddress); err != nil { - return &types.MsgUpdateConsumerResponse{}, errorsmod.Wrapf(types.ErrInvalidNewOwnerAddress, "invalid new owner address %s", msg.NewOwnerAddress) - } - } - ownerAddress, err := k.Keeper.GetConsumerOwnerAddress(ctx, consumerId) if err != nil { return &types.MsgUpdateConsumerResponse{}, errorsmod.Wrapf(types.ErrNoOwnerAddress, "cannot retrieve owner address %s", ownerAddress) @@ -376,12 +368,18 @@ func (k msgServer) UpdateConsumer(goCtx context.Context, msg *types.MsgUpdateCon return &types.MsgUpdateConsumerResponse{}, errorsmod.Wrapf(types.ErrUnauthorized, "expected owner address %s, got %s", ownerAddress, msg.Signer) } + // 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) != "" { + if _, err := k.accountKeeper.AddressCodec().StringToBytes(msg.NewOwnerAddress); err != nil { + return &types.MsgUpdateConsumerResponse{}, errorsmod.Wrapf(types.ErrInvalidNewOwnerAddress, "invalid new owner address %s", msg.NewOwnerAddress) + } + k.Keeper.SetConsumerOwnerAddress(ctx, consumerId, msg.NewOwnerAddress) } if msg.Metadata != nil { - if err := k.SetConsumerMetadata(ctx, consumerId, *msg.Metadata); err != nil { + if err := k.Keeper.SetConsumerMetadata(ctx, consumerId, *msg.Metadata); err != nil { return &types.MsgUpdateConsumerResponse{}, errorsmod.Wrapf(types.ErrInvalidConsumerMetadata, "cannot set consumer metadata: %s", err.Error()) } @@ -389,7 +387,8 @@ func (k msgServer) UpdateConsumer(goCtx context.Context, msg *types.MsgUpdateCon // get the previous spawn time so that we can use it in `PrepareConsumerForLaunch` var previousSpawnTime time.Time - if previousInitializationParameters, err := k.Keeper.GetConsumerInitializationParameters(ctx, msg.ConsumerId); err != nil { + previousInitializationParameters, err := k.Keeper.GetConsumerInitializationParameters(ctx, msg.ConsumerId) + if err == nil { previousSpawnTime = previousInitializationParameters.SpawnTime } @@ -410,7 +409,7 @@ func (k msgServer) UpdateConsumer(goCtx context.Context, msg *types.MsgUpdateCon } oldTopN := k.Keeper.GetTopN(ctx, consumerId) - if err = k.SetConsumerPowerShapingParameters(ctx, consumerId, *msg.PowerShapingParameters); err != nil { + if err = k.Keeper.SetConsumerPowerShapingParameters(ctx, consumerId, *msg.PowerShapingParameters); err != nil { return &types.MsgUpdateConsumerResponse{}, errorsmod.Wrapf(types.ErrInvalidPowerShapingParameters, "cannot set power shaping parameters") } @@ -426,12 +425,12 @@ func (k msgServer) UpdateConsumer(goCtx context.Context, msg *types.MsgUpdateCon // A Top N cannot change its owner address to something different from the gov module if the chain // remains a Top N chain. - currentOwnerAddress, err := k.GetConsumerOwnerAddress(ctx, consumerId) + currentOwnerAddress, err := k.Keeper.GetConsumerOwnerAddress(ctx, consumerId) if err != nil { return &types.MsgUpdateConsumerResponse{}, errorsmod.Wrapf(types.ErrNoOwnerAddress, "cannot retrieve owner address %s: %s", ownerAddress, err.Error()) } - currentPowerShapingParameters, err := k.GetConsumerPowerShapingParameters(ctx, consumerId) + currentPowerShapingParameters, err := k.Keeper.GetConsumerPowerShapingParameters(ctx, consumerId) if err != nil { return &types.MsgUpdateConsumerResponse{}, errorsmod.Wrapf(types.ErrInvalidPowerShapingParameters, "cannot retrieve power shaping parameters: %s", err.Error()) } @@ -441,9 +440,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.CanLaunch(ctx, consumerId); canLaunch { - k.SetConsumerPhase(ctx, consumerId, Initialized) - k.PrepareConsumerForLaunch(ctx, consumerId, previousSpawnTime, spawnTime) + if spawnTime, canLaunch := k.Keeper.CanLaunch(ctx, consumerId); canLaunch { + k.Keeper.SetConsumerPhase(ctx, consumerId, Initialized) + k.Keeper.PrepareConsumerForLaunch(ctx, consumerId, previousSpawnTime, spawnTime) } return &types.MsgUpdateConsumerResponse{}, nil diff --git a/x/ccv/provider/keeper/msg_server_test.go b/x/ccv/provider/keeper/msg_server_test.go index 9dcf3942fe..075a1308bd 100644 --- a/x/ccv/provider/keeper/msg_server_test.go +++ b/x/ccv/provider/keeper/msg_server_test.go @@ -1,11 +1,13 @@ package keeper_test import ( + "github.com/cosmos/cosmos-sdk/codec/address" 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" "github.com/stretchr/testify/require" "testing" + "time" ) func TestCreateConsumer(t *testing.T) { @@ -53,3 +55,113 @@ func TestCreateConsumer(t *testing.T) { require.True(t, found) require.Equal(t, providerkeeper.Registered, phase) } + +func TestUpdateConsumer(t *testing.T) { + providerKeeper, ctx, ctrl, mocks := testkeeper.GetProviderKeeperAndCtx(t, testkeeper.NewInMemKeeperParams(t)) + defer ctrl.Finish() + + msgServer := providerkeeper.NewMsgServerImpl(&providerKeeper) + + // try to update a non-existing (i.e., no consumer id exists) + _, err := msgServer.UpdateConsumer(ctx, + &providertypes.MsgUpdateConsumer{Signer: "signer", ConsumerId: "0", NewOwnerAddress: "cosmos1dkas8mu4kyhl5jrh4nzvm65qz588hy9qcz08la", + Metadata: nil, + InitializationParameters: nil, + PowerShapingParameters: nil, + }) + require.Error(t, err, "cannot update consumer chain") + + // create a chain before updating it + createConsumerResponse, err := msgServer.CreateConsumer(ctx, + &providertypes.MsgCreateConsumer{Signer: "signer", ChainId: "chainId", + Metadata: providertypes.ConsumerMetadata{ + Name: "name", + Description: "description", + Metadata: "metadata", + }, + InitializationParameters: nil, + PowerShapingParameters: nil, + }) + require.NoError(t, err) + consumerId := createConsumerResponse.ConsumerId + + mocks.MockAccountKeeper.EXPECT().AddressCodec().Return(address.NewBech32Codec("cosmos")).AnyTimes() + _, err = msgServer.UpdateConsumer(ctx, + &providertypes.MsgUpdateConsumer{Signer: "wrong signer", ConsumerId: consumerId, NewOwnerAddress: "cosmos1dkas8mu4kyhl5jrh4nzvm65qz588hy9qcz08la", + Metadata: nil, + InitializationParameters: nil, + PowerShapingParameters: nil, + }) + require.Error(t, err, "expected owner address") + + expectedConsumerMetadata := providertypes.ConsumerMetadata{ + Name: "name2", + Description: "description2", + Metadata: "metadata2", + } + + expectedInitializationParameters := testkeeper.GetTestInitializationParameters() + expectedPowerShapingParameters := testkeeper.GetTestPowerShapingParameters() + + expectedOwnerAddress := "cosmos1dkas8mu4kyhl5jrh4nzvm65qz588hy9qcz08la" + _, err = msgServer.UpdateConsumer(ctx, + &providertypes.MsgUpdateConsumer{Signer: "signer", ConsumerId: consumerId, NewOwnerAddress: expectedOwnerAddress, + Metadata: &expectedConsumerMetadata, + InitializationParameters: &expectedInitializationParameters, + PowerShapingParameters: &expectedPowerShapingParameters}) + require.NoError(t, err) + + // assert that owner address was updated + ownerAddress, err := providerKeeper.GetConsumerOwnerAddress(ctx, consumerId) + require.NoError(t, err) + require.Equal(t, expectedOwnerAddress, ownerAddress) + + // assert that consumer metadata were updated + actualConsumerMetadata, err := providerKeeper.GetConsumerMetadata(ctx, consumerId) + require.NoError(t, err) + require.Equal(t, expectedConsumerMetadata, actualConsumerMetadata) + + // assert that initialization parameters were updated + actualInitializationParameters, err := providerKeeper.GetConsumerInitializationParameters(ctx, consumerId) + require.NoError(t, err) + require.Equal(t, expectedInitializationParameters, actualInitializationParameters) + + // assert that power-shaping parameters were updated + actualPowerShapingParameters, err := providerKeeper.GetConsumerPowerShapingParameters(ctx, consumerId) + require.NoError(t, err) + require.Equal(t, expectedPowerShapingParameters, actualPowerShapingParameters) + + // assert phase + phase, found := providerKeeper.GetConsumerPhase(ctx, consumerId) + require.True(t, found) + require.Equal(t, providerkeeper.Initialized, phase) + + // assert that chain is set to launch + consumerIds, err := providerKeeper.GetConsumersToBeLaunched(ctx, expectedInitializationParameters.SpawnTime) + require.NoError(t, err) + require.Equal(t, providertypes.ConsumerIds{ + Ids: []string{consumerId}, + }, consumerIds) + + // re-update (change spawnTime) and verify that the chain is still to be launched at the new spawn time + previousSpawnTime := expectedInitializationParameters.SpawnTime + updatedSpawnTime := expectedInitializationParameters.SpawnTime.Add(time.Hour) + expectedInitializationParameters.SpawnTime = updatedSpawnTime + _, err = msgServer.UpdateConsumer(ctx, + &providertypes.MsgUpdateConsumer{Signer: expectedOwnerAddress, ConsumerId: consumerId, + Metadata: &expectedConsumerMetadata, + InitializationParameters: &expectedInitializationParameters, + PowerShapingParameters: &expectedPowerShapingParameters}) + require.NoError(t, err) + + consumerIds, err = providerKeeper.GetConsumersToBeLaunched(ctx, previousSpawnTime) + require.NoError(t, err) + require.Empty(t, consumerIds) + + consumerIds, err = providerKeeper.GetConsumersToBeLaunched(ctx, updatedSpawnTime) + require.NoError(t, err) + require.Equal(t, providertypes.ConsumerIds{ + Ids: []string{consumerId}, + }, consumerIds) + +} diff --git a/x/ccv/provider/keeper/permissionless.go b/x/ccv/provider/keeper/permissionless.go index b742485f6e..dbe25b5088 100644 --- a/x/ccv/provider/keeper/permissionless.go +++ b/x/ccv/provider/keeper/permissionless.go @@ -1,11 +1,9 @@ package keeper import ( - "bytes" errorsmod "cosmossdk.io/errors" storetypes "cosmossdk.io/store/types" "encoding/binary" - "encoding/gob" "fmt" sdk "github.com/cosmos/cosmos-sdk/types" "github.com/cosmos/interchain-security/v5/x/ccv/provider/types" @@ -43,7 +41,7 @@ func (k Keeper) setConsumerId(ctx sdk.Context, consumerId uint64) { store.Set(types.ConsumerIdKey(), buf) } -// GetConsumerId returns the last registered consumer id +// GetConsumerId returns the next to-be-assigned consumer id func (k Keeper) GetConsumerId(ctx sdk.Context) (uint64, bool) { store := ctx.KVStore(k.storeKey) buf := store.Get(types.ConsumerIdKey()) @@ -151,11 +149,11 @@ func (k Keeper) GetConsumerInitializationParameters(ctx sdk.Context, consumerId } // SetConsumerInitializationParameters sets the initialization parameters associated with this consumer id -func (k Keeper) SetConsumerInitializationParameters(ctx sdk.Context, consumerId string, record types.ConsumerInitializationParameters) error { +func (k Keeper) SetConsumerInitializationParameters(ctx sdk.Context, consumerId string, parameters types.ConsumerInitializationParameters) error { store := ctx.KVStore(k.storeKey) - bz, err := record.Marshal() + bz, err := parameters.Marshal() if err != nil { - return fmt.Errorf("failed to marshal initialization record (%+v) for consumer id (%s): %w", record, consumerId, err) + return fmt.Errorf("failed to marshal initialization parameters (%+v) for consumer id (%s): %w", parameters, consumerId, err) } store.Set(types.ConsumerIdToInitializationParametersKey(consumerId), bz) return nil @@ -209,7 +207,6 @@ func (k Keeper) GetConsumerPhase(ctx sdk.Context, consumerId string) (ConsumerPh } // SetConsumerPhase sets the phase associated with this consumer id -// TODO (PERMISSIONLESS): use this method when launching and when stopping a chain func (k Keeper) SetConsumerPhase(ctx sdk.Context, consumerId string, phase ConsumerPhase) { store := ctx.KVStore(k.storeKey) store.Set(types.ConsumerIdToPhaseKey(consumerId), []byte{byte(phase)}) @@ -252,10 +249,10 @@ func (k Keeper) DeleteConsumerStopTime(ctx sdk.Context, consumerId string) { store.Delete(types.ConsumerIdToStopTimeKey(consumerId)) } -// GetConsumersToBeLaunched -func (k Keeper) GetConsumersToBeLaunched(ctx sdk.Context, spawnTime time.Time) (types.ConsumerIds, error) { +// 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(types.SpawnTimeToConsumerIdsKey(spawnTime)) + bz := store.Get(key(time)) if bz == nil { return types.ConsumerIds{}, nil } @@ -268,233 +265,181 @@ func (k Keeper) GetConsumersToBeLaunched(ctx sdk.Context, spawnTime time.Time) ( return consumerIds, nil } -// AppendSpawnTimeForConsumerToBeLaunched -func (k Keeper) AppendSpawnTimeForConsumerToBeLaunched(ctx sdk.Context, consumerId string, spawnTime time.Time) error { +// 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) - consumerIdsSlice, err := k.GetConsumersToBeLaunched(ctx, spawnTime) + consumers, err := k.getConsumerIdsBasedOnTime(ctx, key, time) if err != nil { return err } - consumerIds := append(consumerIdsSlice.Ids, consumerId) - appendedConsumerIdsStr := types.ConsumerIds{ - Ids: consumerIds, + consumersWithAppend := types.ConsumerIds{ + Ids: append(consumers.Ids, consumerId), } - bz, err := appendedConsumerIdsStr.Marshal() + bz, err := consumersWithAppend.Marshal() if err != nil { return err } - store.Set(types.SpawnTimeToConsumerIdsKey(spawnTime), bz) + store.Set(key(time), bz) return nil } -// RemoveConsumerFromToBeLaunchedConsumers -func (k Keeper) RemoveConsumerFromToBeLaunchedConsumers(ctx sdk.Context, consumerId string, spawnTime time.Time) error { +// 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) - consumerIds, err := k.GetConsumersToBeLaunched(ctx, spawnTime) + consumers, err := k.getConsumerIdsBasedOnTime(ctx, key, time) if err != nil { return err } - if len(consumerIds.Ids) == 0 { - return fmt.Errorf("no consumer ids for spawn time: %s", spawnTime.String()) + 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 := 0 - for i := 0; i < len(consumerIds.Ids); i = i + 1 { - if consumerIds.Ids[i] == consumerId { + index := -1 + for i := 0; i < len(consumers.Ids); i = i + 1 { + if consumers.Ids[i] == consumerId { index = i break } } - if consumerIds.Ids[index] != consumerId { - return fmt.Errorf("failed to find consumer id (%s) in the chains to be launched", consumerId) + + if index == -1 { + return fmt.Errorf("failed to find consumer id (%s)", consumerId) } - if len(consumerIds.Ids) == 1 { - store.Delete(types.SpawnTimeToConsumerIdsKey(spawnTime)) + if len(consumers.Ids) == 1 { + store.Delete(key(time)) return nil } - updatedConsumerIds := append(consumerIds.Ids[:index], consumerIds.Ids[index+1:]...) - - updatedConsumerIdsStr := types.ConsumerIds{ - Ids: updatedConsumerIds, + consumersWithRemoval := types.ConsumerIds{ + Ids: append(consumers.Ids[:index], consumers.Ids[index+1:]...), } - bz, err := updatedConsumerIdsStr.Marshal() + bz, err := consumersWithRemoval.Marshal() if err != nil { return err } - store.Set(types.SpawnTimeToConsumerIdsKey(spawnTime), bz) + store.Set(key(time), bz) return nil } -// TODO (PERMISSIONLESS) merge the functions, they practically do the same - -// GetConsumersToBeStopped -func (k Keeper) GetConsumersToBeStopped(ctx sdk.Context, stopTime time.Time) (types.ConsumerIds, error) { - store := ctx.KVStore(k.storeKey) - bz := store.Get(types.StopTimeToConsumerIdsKey(stopTime)) - if bz == nil { - return types.ConsumerIds{}, nil - } - - var consumerIds types.ConsumerIds - err := consumerIds.Unmarshal(bz) - if err != nil { - return types.ConsumerIds{}, err - } - return consumerIds, 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) } -// AppendSpawnTimeForConsumerToBeStopped -func (k Keeper) AppendStopTimeForConsumerToBeStopped(ctx sdk.Context, consumerId string, stopTime time.Time) error { - store := ctx.KVStore(k.storeKey) - - consumerIdsStr, err := k.GetConsumersToBeStopped(ctx, stopTime) - if err != nil { - return err - } - consumerIds := append(consumerIdsStr.Ids, consumerId) - - consumerIdsNewStr := types.ConsumerIds{ - Ids: consumerIds, - } - - bz, err := consumerIdsNewStr.Marshal() - if err != nil { - return err - } - - store.Set(types.StopTimeToConsumerIdsKey(stopTime), bz) - return nil +// AppendConsumerToBeLaunchedOnSpawnTime appends the provider consumer id for the given spawn time +func (k Keeper) AppendConsumerToBeLaunchedOnSpawnTime(ctx sdk.Context, consumerId string, spawnTime time.Time) error { + return k.appendConsumerIdOnTime(ctx, consumerId, types.SpawnTimeToConsumerIdsKey, spawnTime) } -// RemoveConsumerFromToBeStoppedConsumers -func (k Keeper) RemoveConsumerFromToBeStoppedConsumers(ctx sdk.Context, consumerId string, stopTime time.Time) error { - store := ctx.KVStore(k.storeKey) - - consumerIds, err := k.GetConsumersToBeStopped(ctx, stopTime) - if err != nil { - return err - } - - if len(consumerIds.Ids) == 0 { - return fmt.Errorf("no consumer ids for stop time: %s", stopTime.String()) - } - - // find the index of the consumer we want to remove - index := 0 - for i := 0; i < len(consumerIds.Ids); i = i + 1 { - if consumerIds.Ids[i] == consumerId { - index = i - break - } - } - if consumerIds.Ids[index] != consumerId { - return fmt.Errorf("failed to find consumer id (%s) in the chains to be stopped", consumerId) - } +// RemoveConsumerToBeLaunchedFromSpawnTime removes consumer id from if stored for this specific spawn time +func (k Keeper) RemoveConsumerToBeLaunchedFromSpawnTime(ctx sdk.Context, consumerId string, spawnTime time.Time) error { + return k.removeConsumerIdFromTime(ctx, consumerId, types.SpawnTimeToConsumerIdsKey, spawnTime) +} - if len(consumerIds.Ids) == 1 { - store.Delete(types.StopTimeToConsumerIdsKey(stopTime)) - return nil - } +// 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) +} - updatedConsumerIds := append(consumerIds.Ids[:index], consumerIds.Ids[index+1:]...) - updatedConsumerIdsStr := types.ConsumerIds{ - Ids: updatedConsumerIds, - } - bz, err := updatedConsumerIdsStr.Marshal() - if err != nil { - return err - } +// AppendConsumerToBeStoppedOnStopTime appends the provider consumer id for the given stop time +func (k Keeper) AppendConsumerToBeStoppedOnStopTime(ctx sdk.Context, consumerId string, stopTime time.Time) error { + return k.appendConsumerIdOnTime(ctx, consumerId, types.StopTimeToConsumerIdsKey, stopTime) +} - store.Set(types.StopTimeToConsumerIdsKey(stopTime), bz) - return nil +// RemoveConsumerToBeStoppedFromStopTime removes consumer id from if stored for this specific stop time +func (k Keeper) RemoveConsumerToBeStoppedFromStopTime(ctx sdk.Context, consumerId string, stopTime time.Time) error { + return k.removeConsumerIdFromTime(ctx, consumerId, types.StopTimeToConsumerIdsKey, stopTime) } -// GetOptedInConsumerIds -func (k Keeper) GetOptedInConsumerIds(ctx sdk.Context, providerAddr types.ProviderConsAddress) ([]string, error) { +// 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) bz := store.Get(types.ProviderConsAddrToOptedInConsumerIdsKey(providerAddr)) if bz == nil { - return []string{}, 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) } - var consumerIds []string - buf := bytes.NewBuffer(bz) - dec := gob.NewDecoder(buf) - err := dec.Decode(&consumerIds) - return consumerIds, err + return consumerIds, nil } -// AppendOptedInConsumerId +// AppendOptedInConsumerId appends given consumer id to the list of consumers that validator has opted in func (k Keeper) AppendOptedInConsumerId(ctx sdk.Context, providerAddr types.ProviderConsAddress, consumerId string) error { store := ctx.KVStore(k.storeKey) - consumerIds, err := k.GetOptedInConsumerIds(ctx, providerAddr) + consumers, err := k.GetOptedInConsumerIds(ctx, providerAddr) if err != nil { return err } - consumerIds = append(consumerIds, consumerId) - var buf bytes.Buffer - enc := gob.NewEncoder(&buf) - err = enc.Encode(consumerIds) + consumersWithAppend := types.ConsumerIds{ + Ids: append(consumers.Ids, consumerId), + } + + bz, err := consumersWithAppend.Marshal() if err != nil { return err } - store.Set(types.ProviderConsAddrToOptedInConsumerIdsKey(providerAddr), buf.Bytes()) + store.Set(types.ProviderConsAddrToOptedInConsumerIdsKey(providerAddr), bz) return nil } -// RemoveOptedInConsumerId +// RemoveOptedInConsumerId removes the consumer id from this validator because it is not opted in anymore func (k Keeper) RemoveOptedInConsumerId(ctx sdk.Context, providerAddr types.ProviderConsAddress, consumerId string) error { store := ctx.KVStore(k.storeKey) - consumerIds, err := k.GetOptedInConsumerIds(ctx, providerAddr) + consumers, err := k.GetOptedInConsumerIds(ctx, providerAddr) if err != nil { return err } - if len(consumerIds) == 0 { - return fmt.Errorf("no consumer ids for provider consensus address: %s", providerAddr.String()) + if len(consumers.Ids) == 0 { + return fmt.Errorf("no consumer ids found for this provviderAddr: %s", providerAddr.String()) } // find the index of the consumer we want to remove - index := 0 - for i := 0; i < len(consumerIds); i = i + 1 { - if consumerIds[i] == consumerId { + index := -1 + for i := 0; i < len(consumers.Ids); i = i + 1 { + if consumers.Ids[i] == consumerId { index = i break } } - if consumerIds[index] != consumerId { - return fmt.Errorf("failed to find consumer id (%s) from the opted-in chains", consumerId) + + if index == -1 { + return fmt.Errorf("failed to find consumer id (%s)", consumerId) } - if len(consumerIds) == 1 { + if len(consumers.Ids) == 1 { store.Delete(types.ProviderConsAddrToOptedInConsumerIdsKey(providerAddr)) return nil } - updatedConsumerIds := append(consumerIds[:index], consumerIds[index+1:]...) - var buf bytes.Buffer - enc := gob.NewEncoder(&buf) - err = enc.Encode(updatedConsumerIds) + consumersWithRemoval := types.ConsumerIds{ + Ids: append(consumers.Ids[:index], consumers.Ids[index+1:]...), + } + + bz, err := consumersWithRemoval.Marshal() if err != nil { return err } - store.Set(types.ProviderConsAddrToOptedInConsumerIdsKey(providerAddr), buf.Bytes()) + store.Set(types.ProviderConsAddrToOptedInConsumerIdsKey(providerAddr), bz) return nil } @@ -683,7 +628,7 @@ func (k Keeper) GetLaunchedConsumersReadyToStop(ctx sdk.Context, limit uint32) [ // 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`. func (k Keeper) IsValidatorOptedInToChainId(ctx sdk.Context, providerAddr types.ProviderConsAddress, chainId string) (string, bool) { - consumerIds, err := k.GetOptedInConsumerIds(ctx, providerAddr) + consumers, err := k.GetOptedInConsumerIds(ctx, providerAddr) if err != nil { k.Logger(ctx).Error("failed to retrieve the consumer ids this validator is opted in to", "providerAddr", providerAddr, @@ -691,7 +636,7 @@ func (k Keeper) IsValidatorOptedInToChainId(ctx sdk.Context, providerAddr types. return "", false } - for _, consumerId := range consumerIds { + for _, consumerId := range consumers.Ids { consumerChainId, err := k.GetConsumerChainId(ctx, consumerId) if err != nil { k.Logger(ctx).Error("cannot find chain id", @@ -709,12 +654,15 @@ func (k Keeper) IsValidatorOptedInToChainId(ctx sdk.Context, providerAddr types. } func (k Keeper) PrepareConsumerForLaunch(ctx sdk.Context, consumerId string, previousSpawnTime time.Time, spawnTime time.Time) { + fmt.Printf("previousSpawnTime: \n(%+v)\n", previousSpawnTime) + fmt.Printf("time.Time: \n(%+v)\n", time.Time{}) if !previousSpawnTime.Equal(time.Time{}) { + fmt.Println("mpika edo") // if this is not the first initialization and hence `previousSpawnTime` does not contain the zero value of `Time` - // remove the consumer id from the old spawn time - k.RemoveConsumerFromToBeLaunchedConsumers(ctx, consumerId, previousSpawnTime) + // remove the consumer id from the previous spawn time + k.RemoveConsumerToBeLaunchedFromSpawnTime(ctx, consumerId, previousSpawnTime) } - k.AppendSpawnTimeForConsumerToBeLaunched(ctx, consumerId, spawnTime) + k.AppendConsumerToBeLaunchedOnSpawnTime(ctx, consumerId, spawnTime) } // CanLaunch checks on whether the consumer with `consumerId` has set all the initialization parameters set and hence diff --git a/x/ccv/provider/keeper/permissionless_test.go b/x/ccv/provider/keeper/permissionless_test.go index 5819e355d0..34458ce0d0 100644 --- a/x/ccv/provider/keeper/permissionless_test.go +++ b/x/ccv/provider/keeper/permissionless_test.go @@ -13,160 +13,220 @@ import ( "time" ) -func TestFetchAndIncrementConsumerId(t *testing.T) { +// TestConsumerId tests setters and getters of consumer id (i.e., `FetchAndIncrementConsumerId` and `GetConsumerId`) +func TestConsumerId(t *testing.T) { providerKeeper, ctx, ctrl, _ := testkeeper.GetProviderKeeperAndCtx(t, testkeeper.NewInMemKeeperParams(t)) defer ctrl.Finish() + _, found := providerKeeper.GetConsumerId(ctx) + require.False(t, found) + consumerId := providerKeeper.FetchAndIncrementConsumerId(ctx) require.Equal(t, "0", consumerId) + consumerIdNum, found := providerKeeper.GetConsumerId(ctx) + require.Equal(t, uint64(1), consumerIdNum) + require.True(t, found) consumerId = providerKeeper.FetchAndIncrementConsumerId(ctx) require.Equal(t, "1", consumerId) + consumerIdNum, found = providerKeeper.GetConsumerId(ctx) + require.Equal(t, uint64(2), consumerIdNum) + require.True(t, found) +} - consumerId = providerKeeper.FetchAndIncrementConsumerId(ctx) - require.Equal(t, "2", consumerId) +// TestConsumerChainId tests the getter, setter, and deletion of the consumer to chain id methods +func TestConsumerChainId(t *testing.T) { + providerKeeper, ctx, ctrl, _ := testkeeper.GetProviderKeeperAndCtx(t, testkeeper.NewInMemKeeperParams(t)) + defer ctrl.Finish() + + _, err := providerKeeper.GetConsumerChainId(ctx, "chainId") + require.Error(t, err, "failed to retrieve chain id") + + providerKeeper.SetConsumerChainId(ctx, "chainId", "chainId") + chainId, err := providerKeeper.GetConsumerChainId(ctx, "chainId") + require.NoError(t, err) + require.Equal(t, "chainId", chainId) + + // write under a different key + providerKeeper.SetConsumerChainId(ctx, "consumerId2", "chainId") + chainId, err = providerKeeper.GetConsumerChainId(ctx, "consumerId2") + require.NoError(t, err) + require.Equal(t, "chainId", chainId) + + // assert that overwriting the current key works + providerKeeper.SetConsumerChainId(ctx, "chainId", "chainId2") + chainId, err = providerKeeper.GetConsumerChainId(ctx, "chainId") + require.NoError(t, err) + require.Equal(t, "chainId2", chainId) + + providerKeeper.DeleteConsumerChainId(ctx, "chainId") + _, err = providerKeeper.GetConsumerChainId(ctx, "chainId") + require.Error(t, err, "failed to retrieve chain id") } -// TestClientIdToConsumerId tests the getter, setter, and deletion methods of the client id to consumer id methods -func TestClientIdToConsumerId(t *testing.T) { +// TestConsumerOwnerAddress tests the getter, setter, and deletion of the consumer to owner address methods +func TestConsumerOwnerAddress(t *testing.T) { providerKeeper, ctx, ctrl, _ := testkeeper.GetProviderKeeperAndCtx(t, testkeeper.NewInMemKeeperParams(t)) defer ctrl.Finish() - _, found := providerKeeper.GetClientIdToConsumerId(ctx, "clientId") - require.False(t, found) + _, err := providerKeeper.GetConsumerOwnerAddress(ctx, "ownerAddress") + require.Error(t, err, "failed to retrieve owner address") - providerKeeper.SetClientIdToConsumerId(ctx, "clientId", "consumerId") - consumerId, found := providerKeeper.GetClientIdToConsumerId(ctx, "clientId") - require.True(t, found) - require.Equal(t, "consumerId", consumerId) + providerKeeper.SetConsumerOwnerAddress(ctx, "consumerId", "owner address") + ownerAddress, err := providerKeeper.GetConsumerOwnerAddress(ctx, "consumerId") + require.NoError(t, err) + require.Equal(t, "owner address", ownerAddress) - // 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) + // write under a different key + providerKeeper.SetConsumerOwnerAddress(ctx, "consumerId2", "owner address") + ownerAddress, err = providerKeeper.GetConsumerOwnerAddress(ctx, "consumerId2") + require.NoError(t, err) + require.Equal(t, "owner address", ownerAddress) - providerKeeper.DeleteClientIdToConsumerId(ctx, "clientId") - consumerId, found = providerKeeper.GetClientIdToConsumerId(ctx, "clientId") - require.False(t, found) - require.Equal(t, "", consumerId) + // assert that overwriting the current key works + providerKeeper.SetConsumerOwnerAddress(ctx, "consumerId", "owner address2") + ownerAddress, err = providerKeeper.GetConsumerOwnerAddress(ctx, "consumerId") + require.NoError(t, err) + require.Equal(t, "owner address2", ownerAddress) + + providerKeeper.DeleteConsumerOwnerAddress(ctx, "consumerId") + _, err = providerKeeper.GetConsumerChainId(ctx, "consumerId") + require.Error(t, err, "failed to retrieve owner address") } -// TestConsumerIdToRegistrationRecord tests the getter, setter, and deletion methods of the consumer id to registration record methods -func TestConsumerIdToRegistrationRecord(t *testing.T) { +// TestConsumerMetadata tests the getter, setter, and deletion of the consumer id to consumer metadata methods +func TestConsumerMetadata(t *testing.T) { providerKeeper, ctx, ctrl, _ := testkeeper.GetProviderKeeperAndCtx(t, testkeeper.NewInMemKeeperParams(t)) defer ctrl.Finish() _, err := providerKeeper.GetConsumerMetadata(ctx, "consumerId") require.Error(t, err) - expectedRecord := providertypes.ConsumerMetadata{ + expectedMetadata := providertypes.ConsumerMetadata{ Name: "name", Description: "description", Metadata: "metadata", //ChainId: "chain_id", } - providerKeeper.SetConsumerMetadata(ctx, "consumerId", expectedRecord) - actualRecord, err := providerKeeper.GetConsumerMetadata(ctx, "consumerId") + providerKeeper.SetConsumerMetadata(ctx, "consumerId", expectedMetadata) + actualMetadata, err := providerKeeper.GetConsumerMetadata(ctx, "consumerId") require.NoError(t, err) - require.Equal(t, expectedRecord, actualRecord) + require.Equal(t, expectedMetadata, actualMetadata) // assert that overwriting the current registration record works - expectedRecord = providertypes.ConsumerMetadata{ + expectedMetadata = providertypes.ConsumerMetadata{ Name: "name 2", Description: "description 2", Metadata: "metadata 2", //ChainId: "chain_id2", } - providerKeeper.SetConsumerMetadata(ctx, "consumerId", expectedRecord) - actualRecord, err = providerKeeper.GetConsumerMetadata(ctx, "consumerId") + providerKeeper.SetConsumerMetadata(ctx, "consumerId", expectedMetadata) + actualMetadata, err = providerKeeper.GetConsumerMetadata(ctx, "consumerId") require.NoError(t, err) - require.Equal(t, expectedRecord, actualRecord) + require.Equal(t, expectedMetadata, actualMetadata) providerKeeper.DeleteConsumerMetadata(ctx, "consumerId") - actualRecord, err = providerKeeper.GetConsumerMetadata(ctx, "consumerId") + actualMetadata, err = providerKeeper.GetConsumerMetadata(ctx, "consumerId") require.Error(t, err) - require.Equal(t, providertypes.ConsumerMetadata{}, actualRecord) + require.Equal(t, providertypes.ConsumerMetadata{}, actualMetadata) } -// TestConsumerIdToInitializationRecord tests the getter, setter, and deletion methods of the consumer id to initialization record methods -func TestConsumerIdToInitializationRecord(t *testing.T) { +// TestConsumerInitializationParameters tests the getter, setter, and deletion of the consumer id to initialization parameters methods +func TestConsumerInitializationParameters(t *testing.T) { providerKeeper, ctx, ctrl, _ := testkeeper.GetProviderKeeperAndCtx(t, testkeeper.NewInMemKeeperParams(t)) defer ctrl.Finish() _, err := providerKeeper.GetConsumerInitializationParameters(ctx, "consumerId") require.Error(t, err) - spawnTime := time.Unix(1, 2).UTC() - unbondingPeriod := time.Duration(3456) - ccvTimeoutPeriod := time.Duration(789) - transferTimeoutPeriod := time.Duration(101112) - expectedRecord := providertypes.ConsumerInitializationParameters{ + expectedInitializationParameters := providertypes.ConsumerInitializationParameters{ InitialHeight: types.Height{RevisionNumber: 1, RevisionHeight: 2}, GenesisHash: []byte{0, 1}, BinaryHash: []byte{2, 3}, - SpawnTime: spawnTime, - UnbondingPeriod: unbondingPeriod, - CcvTimeoutPeriod: ccvTimeoutPeriod, - TransferTimeoutPeriod: transferTimeoutPeriod, + SpawnTime: time.Unix(1, 2).UTC(), + UnbondingPeriod: time.Duration(3456), + CcvTimeoutPeriod: time.Duration(789), + TransferTimeoutPeriod: time.Duration(101112), ConsumerRedistributionFraction: "consumer_redistribution_fraction", BlocksPerDistributionTransmission: 123, HistoricalEntries: 456, DistributionTransmissionChannel: "distribution_transmission_channel", } - providerKeeper.SetConsumerInitializationParameters(ctx, "consumerId", expectedRecord) - actualRecord, err := providerKeeper.GetConsumerInitializationParameters(ctx, "consumerId") + providerKeeper.SetConsumerInitializationParameters(ctx, "consumerId", expectedInitializationParameters) + actualInitializationParameters, err := providerKeeper.GetConsumerInitializationParameters(ctx, "consumerId") require.NoError(t, err) - require.Equal(t, expectedRecord, actualRecord) + require.Equal(t, expectedInitializationParameters, actualInitializationParameters) // assert that overwriting the current initialization record works - spawnTime = time.Unix(2, 3).UTC() - unbondingPeriod = time.Duration(789) - ccvTimeoutPeriod = time.Duration(101112) - transferTimeoutPeriod = time.Duration(131415) - expectedRecord = providertypes.ConsumerInitializationParameters{ + expectedInitializationParameters = providertypes.ConsumerInitializationParameters{ InitialHeight: types.Height{RevisionNumber: 2, RevisionHeight: 3}, GenesisHash: []byte{2, 3}, BinaryHash: []byte{4, 5}, - SpawnTime: spawnTime, - UnbondingPeriod: unbondingPeriod, - CcvTimeoutPeriod: ccvTimeoutPeriod, - TransferTimeoutPeriod: transferTimeoutPeriod, + SpawnTime: time.Unix(2, 3).UTC(), + UnbondingPeriod: time.Duration(789), + CcvTimeoutPeriod: time.Duration(101112), + TransferTimeoutPeriod: time.Duration(131415), ConsumerRedistributionFraction: "consumer_redistribution_fraction2", BlocksPerDistributionTransmission: 456, HistoricalEntries: 789, DistributionTransmissionChannel: "distribution_transmission_channel2", } - providerKeeper.SetConsumerInitializationParameters(ctx, "consumerId", expectedRecord) - actualRecord, err = providerKeeper.GetConsumerInitializationParameters(ctx, "consumerId") + providerKeeper.SetConsumerInitializationParameters(ctx, "consumerId", expectedInitializationParameters) + actualInitializationParameters, err = providerKeeper.GetConsumerInitializationParameters(ctx, "consumerId") require.NoError(t, err) - require.Equal(t, expectedRecord, actualRecord) + require.Equal(t, expectedInitializationParameters, actualInitializationParameters) providerKeeper.DeleteConsumerInitializationParameters(ctx, "consumerId") - actualRecord, err = providerKeeper.GetConsumerInitializationParameters(ctx, "consumerId") + actualInitializationParameters, err = providerKeeper.GetConsumerInitializationParameters(ctx, "consumerId") require.Error(t, err) - require.Equal(t, providertypes.ConsumerInitializationParameters{}, actualRecord) + require.Equal(t, providertypes.ConsumerInitializationParameters{}, actualInitializationParameters) } -// TestConsumerIdToOwnerAddress tests the getter, setter, and deletion methods of the owner address to registration record methods -func TestConsumerIdToOwnerAddress(t *testing.T) { +// TestConsumerPowerShapingParameters tests the getter, setter, and deletion of the consumer id to power-shaping parameters methods +func TestConsumerPowerShapingParameters(t *testing.T) { providerKeeper, ctx, ctrl, _ := testkeeper.GetProviderKeeperAndCtx(t, testkeeper.NewInMemKeeperParams(t)) defer ctrl.Finish() - providerKeeper.SetConsumerOwnerAddress(ctx, "consumerId", "owner_address") - address, err := providerKeeper.GetConsumerOwnerAddress(ctx, "consumerId") + _, err := providerKeeper.GetConsumerPowerShapingParameters(ctx, "consumerId") + require.Error(t, err) + + expectedPowerShapingParameters := providertypes.PowerShapingParameters{ + Top_N: 10, + ValidatorsPowerCap: 34, + ValidatorSetCap: 10, + Allowlist: []string{"allowlist1", "allowlist2"}, + Denylist: []string{"denylist1", "denylist2"}, + MinStake: 234, + AllowInactiveVals: true, + } + providerKeeper.SetConsumerPowerShapingParameters(ctx, "consumerId", expectedPowerShapingParameters) + actualPowerShapingParameters, err := providerKeeper.GetConsumerPowerShapingParameters(ctx, "consumerId") require.NoError(t, err) - require.Equal(t, "owner_address", address) + require.Equal(t, expectedPowerShapingParameters, actualPowerShapingParameters) - // assert that overwriting the current owner address works - providerKeeper.SetConsumerOwnerAddress(ctx, "consumerId", "owner_address2") - address, err = providerKeeper.GetConsumerOwnerAddress(ctx, "consumerId") + // assert that overwriting the current initialization record works + expectedPowerShapingParameters = providertypes.PowerShapingParameters{ + Top_N: 12, + ValidatorsPowerCap: 67, + ValidatorSetCap: 20, + Allowlist: []string{"allowlist3", "allowlist4"}, + Denylist: []string{"denylist3", "denylist4"}, + MinStake: 567, + AllowInactiveVals: false, + } + providerKeeper.SetConsumerPowerShapingParameters(ctx, "consumerId", expectedPowerShapingParameters) + actualPowerShapingParameters, err = providerKeeper.GetConsumerPowerShapingParameters(ctx, "consumerId") require.NoError(t, err) - require.Equal(t, "owner_address2", address) + require.Equal(t, expectedPowerShapingParameters, actualPowerShapingParameters) + + providerKeeper.DeleteConsumerPowerShapingParameters(ctx, "consumerId") + actualPowerShapingParameters, err = providerKeeper.GetConsumerPowerShapingParameters(ctx, "consumerId") + require.Error(t, err) + require.Equal(t, providertypes.PowerShapingParameters{}, actualPowerShapingParameters) } -// TestConsumerIdToPhase tests the getter, setter, and deletion methods of the consumer id to phase methods -func TestConsumerIdToPhase(t *testing.T) { +// TestConsumerPhase tests the getter, setter, and deletion of the consumer id to phase methods +func TestConsumerPhase(t *testing.T) { providerKeeper, ctx, ctrl, _ := testkeeper.GetProviderKeeperAndCtx(t, testkeeper.NewInMemKeeperParams(t)) defer ctrl.Finish() @@ -184,8 +244,8 @@ 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) { +// 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() @@ -203,6 +263,130 @@ func TestConsumerIdToStopTime(t *testing.T) { require.Error(t, err) } +// TestConsumersToBeLaunched tests `AppendConsumerToBeLaunchedOnSpawnTime`, `GetConsumersToBeLaunched`, and `RemoveConsumerToBeLaunchedFromSpawnTime` +func TestConsumersToBeLaunched(t *testing.T) { + providerKeeper, ctx, ctrl, _ := testkeeper.GetProviderKeeperAndCtx(t, testkeeper.NewInMemKeeperParams(t)) + defer ctrl.Finish() + + spawnTime := time.Now() + providerKeeper.AppendConsumerToBeLaunchedOnSpawnTime(ctx, "consumerId1", spawnTime) + consumers, err := providerKeeper.GetConsumersToBeLaunched(ctx, spawnTime) + require.NoError(t, err) + require.Equal(t, []string{"consumerId1"}, consumers.Ids) + + providerKeeper.AppendConsumerToBeLaunchedOnSpawnTime(ctx, "consumerId2", spawnTime) + consumers, err = providerKeeper.GetConsumersToBeLaunched(ctx, spawnTime) + require.NoError(t, err) + require.Equal(t, []string{"consumerId1", "consumerId2"}, consumers.Ids) + + providerKeeper.AppendConsumerToBeLaunchedOnSpawnTime(ctx, "consumerId3", spawnTime) + consumers, err = providerKeeper.GetConsumersToBeLaunched(ctx, spawnTime) + require.NoError(t, err) + require.Equal(t, []string{"consumerId1", "consumerId2", "consumerId3"}, consumers.Ids) + + err = providerKeeper.RemoveConsumerToBeLaunchedFromSpawnTime(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) + providerKeeper.AppendConsumerToBeLaunchedOnSpawnTime(ctx, "consumerId4", spawnTimePlusOneHour) + 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.RemoveConsumerToBeLaunchedFromSpawnTime(ctx, "consumerId3", spawnTime) + require.NoError(t, err) + err = providerKeeper.RemoveConsumerToBeLaunchedFromSpawnTime(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.RemoveConsumerToBeLaunchedFromSpawnTime(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.AppendConsumerToBeLaunchedOnSpawnTime(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 `AppendConsumerToBeLaunchedOnSpawnTime`, `GetConsumersToBeLaunched`, and `RemoveConsumerToBeLaunchedFromSpawnTime` +func TestConsumersToBeStopped(t *testing.T) { + providerKeeper, ctx, ctrl, _ := testkeeper.GetProviderKeeperAndCtx(t, testkeeper.NewInMemKeeperParams(t)) + defer ctrl.Finish() + + stopTime := time.Now() + providerKeeper.AppendConsumerToBeStoppedOnStopTime(ctx, "consumerId1", stopTime) + consumers, err := providerKeeper.GetConsumersToBeStopped(ctx, stopTime) + require.NoError(t, err) + require.Equal(t, []string{"consumerId1"}, consumers.Ids) + + providerKeeper.AppendConsumerToBeStoppedOnStopTime(ctx, "consumerId2", stopTime) + consumers, err = providerKeeper.GetConsumersToBeStopped(ctx, stopTime) + require.NoError(t, err) + require.Equal(t, []string{"consumerId1", "consumerId2"}, consumers.Ids) + + providerKeeper.AppendConsumerToBeStoppedOnStopTime(ctx, "consumerId3", stopTime) + consumers, err = providerKeeper.GetConsumersToBeStopped(ctx, stopTime) + require.NoError(t, err) + require.Equal(t, []string{"consumerId1", "consumerId2", "consumerId3"}, consumers.Ids) + + err = providerKeeper.RemoveConsumerToBeStoppedFromStopTime(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) + providerKeeper.AppendConsumerToBeStoppedOnStopTime(ctx, "consumerId4", stopTimePlusOneHour) + 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.RemoveConsumerToBeStoppedFromStopTime(ctx, "consumerId3", stopTime) + require.NoError(t, err) + err = providerKeeper.RemoveConsumerToBeStoppedFromStopTime(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.RemoveConsumerToBeStoppedFromStopTime(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.AppendConsumerToBeStoppedOnStopTime(ctx, "consumerId5", stopTime) + require.NoError(t, err) + consumers, err = providerKeeper.GetConsumersToBeStopped(ctx, stopTime) + require.NoError(t, err) + require.Equal(t, []string{"consumerId5"}, consumers.Ids) +} + // 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)) @@ -211,9 +395,9 @@ func TestGetInitializedConsumersReadyToLaunch(t *testing.T) { // no chains to-be-launched exist require.Empty(t, providerKeeper.GetInitializedConsumersReadyToLaunch(ctx, 5)) - providerKeeper.AppendSpawnTimeForConsumerToBeLaunched(ctx, "consumerId1", time.Unix(10, 0)) - providerKeeper.AppendSpawnTimeForConsumerToBeLaunched(ctx, "consumerId2", time.Unix(20, 0)) - providerKeeper.AppendSpawnTimeForConsumerToBeLaunched(ctx, "consumerId3", time.Unix(30, 0)) + providerKeeper.AppendConsumerToBeLaunchedOnSpawnTime(ctx, "consumerId1", time.Unix(10, 0)) + providerKeeper.AppendConsumerToBeLaunchedOnSpawnTime(ctx, "consumerId2", time.Unix(20, 0)) + providerKeeper.AppendConsumerToBeLaunchedOnSpawnTime(ctx, "consumerId3", time.Unix(30, 0)) // time has not yet reached the spawn time of "consumerId1" ctx = ctx.WithBlockTime(time.Unix(9, 999999999)) @@ -237,6 +421,31 @@ func TestGetInitializedConsumersReadyToLaunch(t *testing.T) { require.Equal(t, []string{"consumerId1", "consumerId2"}, providerKeeper.GetInitializedConsumersReadyToLaunch(ctx, 2)) } +// 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) +} + func TestGetLaunchedConsumersReadyToStop(t *testing.T) { providerKeeper, ctx, ctrl, _ := testkeeper.GetProviderKeeperAndCtx(t, testkeeper.NewInMemKeeperParams(t)) defer ctrl.Finish() @@ -244,9 +453,9 @@ func TestGetLaunchedConsumersReadyToStop(t *testing.T) { // no chains to-be-stopped exist require.Empty(t, providerKeeper.GetLaunchedConsumersReadyToStop(ctx, 3)) - providerKeeper.AppendStopTimeForConsumerToBeStopped(ctx, "consumerId1", time.Unix(10, 0)) - providerKeeper.AppendStopTimeForConsumerToBeStopped(ctx, "consumerId2", time.Unix(20, 0)) - providerKeeper.AppendStopTimeForConsumerToBeStopped(ctx, "consumerId3", time.Unix(30, 0)) + providerKeeper.AppendConsumerToBeStoppedOnStopTime(ctx, "consumerId1", time.Unix(10, 0)) + providerKeeper.AppendConsumerToBeStoppedOnStopTime(ctx, "consumerId2", time.Unix(20, 0)) + providerKeeper.AppendConsumerToBeStoppedOnStopTime(ctx, "consumerId3", time.Unix(30, 0)) // time has not yet reached the stop time of "consumerId1" ctx = ctx.WithBlockTime(time.Unix(9, 999999999)) @@ -302,7 +511,7 @@ func TestUpdateAllowlist(t *testing.T) { require.Equal(t, expectedAllowlist, providerKeeper.GetAllowList(ctx, consumerId)) } -func TestPopulateDenylist(t *testing.T) { +func TestUpdateDenylist(t *testing.T) { providerKeeper, ctx, ctrl, _ := testkeeper.GetProviderKeeperAndCtx(t, testkeeper.NewInMemKeeperParams(t)) defer ctrl.Finish() @@ -321,7 +530,7 @@ func TestPopulateDenylist(t *testing.T) { require.Equal(t, expectedDenylist, providerKeeper.GetDenyList(ctx, consumerId)) } -func TestPopulateMinimumPowerInTopN(t *testing.T) { +func TestUpdateMinimumPowerInTopN(t *testing.T) { providerKeeper, ctx, ctrl, mocks := testkeeper.GetProviderKeeperAndCtx(t, testkeeper.NewInMemKeeperParams(t)) defer ctrl.Finish() @@ -380,5 +589,4 @@ func TestPopulateMinimumPowerInTopN(t *testing.T) { minimumPowerInTopN, found = providerKeeper.GetMinimumPowerInTopN(ctx, consumerId) require.True(t, found) require.Equal(t, int64(10), minimumPowerInTopN) - } diff --git a/x/ccv/provider/keeper/proposal.go b/x/ccv/provider/keeper/proposal.go index d13ad3d2bd..8361eb0dbb 100644 --- a/x/ccv/provider/keeper/proposal.go +++ b/x/ccv/provider/keeper/proposal.go @@ -45,7 +45,7 @@ func (k Keeper) CreateConsumerClient(ctx sdk.Context, consumerId string) error { phase, found := k.GetConsumerPhase(ctx, consumerId) if !found || phase != Initialized { return errorsmod.Wrapf(types.ErrInvalidPhase, - "cannot create client for consumer chain that is not in the Initialized phase: %s", consumerId) + "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) @@ -94,7 +94,7 @@ func (k Keeper) CreateConsumerClient(ctx sdk.Context, consumerId string) error { k.SetConsumerClientId(ctx, consumerId, clientID) k.SetClientIdToConsumerId(ctx, clientID, consumerId) - k.Logger(ctx).Info("consumer chain registered (client created)", + k.Logger(ctx).Info("consumer chain launched (client created)", "consumer id", consumerId, "client id", clientID, ) @@ -345,7 +345,7 @@ 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.RemoveConsumerFromToBeLaunchedConsumers(ctx, consumerId, record.SpawnTime) + k.RemoveConsumerToBeLaunchedFromSpawnTime(ctx, consumerId, record.SpawnTime) cachedCtx, writeFn := ctx.CacheContext() err = k.LaunchConsumer(cachedCtx, consumerId) @@ -492,7 +492,7 @@ func (k Keeper) BeginBlockCCR(ctx sdk.Context) { ctx.EventManager().EmitEvents(cachedCtx.EventManager().Events()) k.SetConsumerPhase(cachedCtx, consumerId, Stopped) - k.RemoveConsumerFromToBeStoppedConsumers(ctx, consumerId, stopTime) + k.RemoveConsumerToBeStoppedFromStopTime(ctx, consumerId, stopTime) writeFn() k.Logger(ctx).Info("executed consumer removal", diff --git a/x/ccv/provider/keeper/proposal_test.go b/x/ccv/provider/keeper/proposal_test.go index 1aef84957f..ee0f8039cd 100644 --- a/x/ccv/provider/keeper/proposal_test.go +++ b/x/ccv/provider/keeper/proposal_test.go @@ -805,7 +805,7 @@ func TestBeginBlockInit(t *testing.T) { providerKeeper.SetConsumerInitializationParameters(ctx, fmt.Sprintf("%d", i), r) // set up the chains in their initialized phase, hence they could launch providerKeeper.SetConsumerPhase(ctx, fmt.Sprintf("%d", i), providerkeeper.Initialized) - providerKeeper.AppendSpawnTimeForConsumerToBeLaunched(ctx, fmt.Sprintf("%d", i), r.SpawnTime) + providerKeeper.AppendConsumerToBeLaunchedOnSpawnTime(ctx, fmt.Sprintf("%d", i), r.SpawnTime) } for i, r := range powerShapingParameters { providerKeeper.SetConsumerPowerShapingParameters(ctx, fmt.Sprintf("%d", i), r) @@ -875,11 +875,11 @@ 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.AppendStopTimeForConsumerToBeStopped(ctx, consumerIds[0], now.Add(-time.Hour)) + providerKeeper.AppendConsumerToBeStoppedOnStopTime(ctx, consumerIds[0], now.Add(-time.Hour)) providerKeeper.SetConsumerStopTime(ctx, consumerIds[1], now) - providerKeeper.AppendStopTimeForConsumerToBeStopped(ctx, consumerIds[1], now) + providerKeeper.AppendConsumerToBeStoppedOnStopTime(ctx, consumerIds[1], now) providerKeeper.SetConsumerStopTime(ctx, consumerIds[2], now.Add(time.Hour)) - providerKeeper.AppendStopTimeForConsumerToBeStopped(ctx, consumerIds[2], now.Add(time.Hour)) + providerKeeper.AppendConsumerToBeStoppedOnStopTime(ctx, consumerIds[2], now.Add(time.Hour)) // // Mock expectations From a400de13047f833abcf26401abb5583b8735c5c6 Mon Sep 17 00:00:00 2001 From: insumity Date: Mon, 26 Aug 2024 16:47:42 +0200 Subject: [PATCH 2/5] added some additional tests --- x/ccv/provider/keeper/msg_server.go | 10 +- x/ccv/provider/keeper/msg_server_test.go | 1 - x/ccv/provider/keeper/permissionless.go | 105 +++++---- x/ccv/provider/keeper/permissionless_test.go | 212 +++++++++++++++---- x/ccv/provider/keeper/proposal.go | 2 + x/ccv/provider/types/errors.go | 1 + 6 files changed, 234 insertions(+), 97 deletions(-) diff --git a/x/ccv/provider/keeper/msg_server.go b/x/ccv/provider/keeper/msg_server.go index eb9f9b4f75..fb81b0ef12 100644 --- a/x/ccv/provider/keeper/msg_server.go +++ b/x/ccv/provider/keeper/msg_server.go @@ -342,7 +342,10 @@ func (k msgServer) CreateConsumer(goCtx context.Context, msg *types.MsgCreateCon if spawnTime, canLaunch := k.Keeper.CanLaunch(ctx, consumerId); canLaunch { k.Keeper.SetConsumerPhase(ctx, consumerId, Initialized) - k.Keeper.PrepareConsumerForLaunch(ctx, consumerId, time.Time{}, spawnTime) + if err := k.Keeper.PrepareConsumerForLaunch(ctx, consumerId, time.Time{}, spawnTime); err != nil { + return &types.MsgCreateConsumerResponse{}, errorsmod.Wrapf(types.ErrCannotPrepareForLaunch, + "cannot prepare chain with consumer id (%s) for launch", consumerId) + } } return &types.MsgCreateConsumerResponse{ConsumerId: consumerId}, nil @@ -442,7 +445,10 @@ func (k msgServer) UpdateConsumer(goCtx context.Context, msg *types.MsgUpdateCon if spawnTime, canLaunch := k.Keeper.CanLaunch(ctx, consumerId); canLaunch { k.Keeper.SetConsumerPhase(ctx, consumerId, Initialized) - k.Keeper.PrepareConsumerForLaunch(ctx, consumerId, previousSpawnTime, spawnTime) + if err := k.Keeper.PrepareConsumerForLaunch(ctx, consumerId, previousSpawnTime, spawnTime); err != nil { + return &types.MsgUpdateConsumerResponse{}, errorsmod.Wrapf(types.ErrCannotPrepareForLaunch, + "cannot prepare chain with consumer id (%s) for launch", consumerId) + } } return &types.MsgUpdateConsumerResponse{}, nil diff --git a/x/ccv/provider/keeper/msg_server_test.go b/x/ccv/provider/keeper/msg_server_test.go index 075a1308bd..b9e1f05b58 100644 --- a/x/ccv/provider/keeper/msg_server_test.go +++ b/x/ccv/provider/keeper/msg_server_test.go @@ -163,5 +163,4 @@ func TestUpdateConsumer(t *testing.T) { require.Equal(t, providertypes.ConsumerIds{ Ids: []string{consumerId}, }, consumerIds) - } diff --git a/x/ccv/provider/keeper/permissionless.go b/x/ccv/provider/keeper/permissionless.go index dbe25b5088..1c4a6a5597 100644 --- a/x/ccv/provider/keeper/permissionless.go +++ b/x/ccv/provider/keeper/permissionless.go @@ -508,6 +508,48 @@ func (k Keeper) GetInitializedConsumersReadyToLaunch(ctx sdk.Context, limit uint return result } +// GetLaunchedConsumersReadyToStop 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 { + 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(types.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 { @@ -518,16 +560,13 @@ func (k Keeper) LaunchConsumer(ctx sdk.Context, consumerId string) error { consumerGenesis, found := k.GetConsumerGenesis(ctx, consumerId) if !found { - return errorsmod.Wrapf(types.ErrNoConsumerGenesis, "consumer genesis could not be 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") + return errorsmod.Wrapf(types.ErrInvalidConsumerGenesis, "consumer genesis initial validator set is empty - no validators opted in consumer id: %s", consumerId) } - // The cached context is created with a new EventManager so we merge the event - // into the original context - ctx.EventManager().EmitEvents(ctx.EventManager().Events()) return nil } @@ -582,48 +621,6 @@ func (k Keeper) UpdateMinimumPowerInTopN(ctx sdk.Context, consumerId string, old 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, 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(types.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 - } - - consumerIds, 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(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 -} - // 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`. @@ -653,16 +650,18 @@ func (k Keeper) IsValidatorOptedInToChainId(ctx sdk.Context, providerAddr types. return "", false } -func (k Keeper) PrepareConsumerForLaunch(ctx sdk.Context, consumerId string, previousSpawnTime time.Time, spawnTime time.Time) { - fmt.Printf("previousSpawnTime: \n(%+v)\n", previousSpawnTime) - fmt.Printf("time.Time: \n(%+v)\n", time.Time{}) +// 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{}) { - fmt.Println("mpika edo") // 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 - k.RemoveConsumerToBeLaunchedFromSpawnTime(ctx, consumerId, previousSpawnTime) + err := k.RemoveConsumerToBeLaunchedFromSpawnTime(ctx, consumerId, previousSpawnTime) + if err != nil { + return err + } } - k.AppendConsumerToBeLaunchedOnSpawnTime(ctx, consumerId, spawnTime) + return k.AppendConsumerToBeLaunchedOnSpawnTime(ctx, consumerId, spawnTime) } // CanLaunch checks on whether the consumer with `consumerId` has set all the initialization parameters set and hence diff --git a/x/ccv/provider/keeper/permissionless_test.go b/x/ccv/provider/keeper/permissionless_test.go index 34458ce0d0..7e290dea5b 100644 --- a/x/ccv/provider/keeper/permissionless_test.go +++ b/x/ccv/provider/keeper/permissionless_test.go @@ -387,38 +387,58 @@ func TestConsumersToBeStopped(t *testing.T) { require.Equal(t, []string{"consumerId5"}, consumers.Ids) } -// TestGetInitializedConsumersReadyToLaunch tests that the ready to-be-launched consumer chains are returned -func TestGetInitializedConsumersReadyToLaunch(t *testing.T) { +// TestOptedInConsumerIds tests the `GetOptedInConsumerIds`, `AppendOptedInConsumerId`, and `RemoveOptedInConsumerId` methods +func TestGetOptedInConsumerIds(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)) + providerAddr := providertypes.NewProviderConsAddress([]byte("providerAddr")) + consumers, err := providerKeeper.GetOptedInConsumerIds(ctx, providerAddr) + require.NoError(t, err) + require.Empty(t, consumers) - providerKeeper.AppendConsumerToBeLaunchedOnSpawnTime(ctx, "consumerId1", time.Unix(10, 0)) - providerKeeper.AppendConsumerToBeLaunchedOnSpawnTime(ctx, "consumerId2", time.Unix(20, 0)) - providerKeeper.AppendConsumerToBeLaunchedOnSpawnTime(ctx, "consumerId3", time.Unix(30, 0)) + err = providerKeeper.AppendOptedInConsumerId(ctx, providerAddr, "consumerId1") + require.NoError(t, err) + consumers, err = providerKeeper.GetOptedInConsumerIds(ctx, providerAddr) + require.NoError(t, err) + require.Equal(t, providertypes.ConsumerIds{ + Ids: []string{"consumerId1"}, + }, consumers) - // time has not yet reached the spawn time of "consumerId1" - ctx = ctx.WithBlockTime(time.Unix(9, 999999999)) - require.Empty(t, providerKeeper.GetInitializedConsumersReadyToLaunch(ctx, 3)) + err = providerKeeper.AppendOptedInConsumerId(ctx, providerAddr, "consumerId2") + require.NoError(t, err) + consumers, err = providerKeeper.GetOptedInConsumerIds(ctx, providerAddr) + require.NoError(t, err) + require.Equal(t, providertypes.ConsumerIds{ + Ids: []string{"consumerId1", "consumerId2"}, + }, consumers) - // time has reached the spawn time of "consumerId1" - ctx = ctx.WithBlockTime(time.Unix(10, 0)) - require.Equal(t, []string{"consumerId1"}, providerKeeper.GetInitializedConsumersReadyToLaunch(ctx, 3)) + err = providerKeeper.AppendOptedInConsumerId(ctx, providerAddr, "consumerId3") + require.NoError(t, err) + consumers, err = providerKeeper.GetOptedInConsumerIds(ctx, providerAddr) + require.NoError(t, err) + require.Equal(t, providertypes.ConsumerIds{ + Ids: []string{"consumerId1", "consumerId2", "consumerId3"}, + }, consumers) - // 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)) + // remove all the consumer ids + err = providerKeeper.RemoveOptedInConsumerId(ctx, providerAddr, "consumerId2") + require.NoError(t, err) + consumers, err = providerKeeper.GetOptedInConsumerIds(ctx, providerAddr) + require.NoError(t, err) + require.Equal(t, providertypes.ConsumerIds{ + Ids: []string{"consumerId1", "consumerId3"}, + }, consumers) - // 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)) + err = providerKeeper.RemoveOptedInConsumerId(ctx, providerAddr, "consumerId3") + require.NoError(t, err) - // 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)) + err = providerKeeper.RemoveOptedInConsumerId(ctx, providerAddr, "consumerId1") + require.NoError(t, err) + + consumers, err = providerKeeper.GetOptedInConsumerIds(ctx, providerAddr) + require.NoError(t, err) + require.Empty(t, consumers) } // TestConsumerChainId tests the getter, setter, and deletion of the client id to consumer id methods @@ -446,6 +466,40 @@ func TestClientIdToConsumerId(t *testing.T) { 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)) + defer ctrl.Finish() + + // no chains to-be-launched exist + require.Empty(t, providerKeeper.GetInitializedConsumersReadyToLaunch(ctx, 5)) + + providerKeeper.AppendConsumerToBeLaunchedOnSpawnTime(ctx, "consumerId1", time.Unix(10, 0)) + providerKeeper.AppendConsumerToBeLaunchedOnSpawnTime(ctx, "consumerId2", time.Unix(20, 0)) + providerKeeper.AppendConsumerToBeLaunchedOnSpawnTime(ctx, "consumerId3", time.Unix(30, 0)) + + // time has not yet reached the spawn time of "consumerId1" + ctx = ctx.WithBlockTime(time.Unix(9, 999999999)) + require.Empty(t, providerKeeper.GetInitializedConsumersReadyToLaunch(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)) + + // 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)) + + // 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)) + + // 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)) +} + func TestGetLaunchedConsumersReadyToStop(t *testing.T) { providerKeeper, ctx, ctrl, _ := testkeeper.GetProviderKeeperAndCtx(t, testkeeper.NewInMemKeeperParams(t)) defer ctrl.Finish() @@ -474,24 +528,6 @@ func TestGetLaunchedConsumersReadyToStop(t *testing.T) { require.Equal(t, []string{"consumerId1", "consumerId2", "consumerId3"}, providerKeeper.GetLaunchedConsumersReadyToStop(ctx, 3)) } -func TestIsValidatorOptedInToChain(t *testing.T) { - providerKeeper, ctx, ctrl, _ := testkeeper.GetProviderKeeperAndCtx(t, testkeeper.NewInMemKeeperParams(t)) - defer ctrl.Finish() - - chainId := "chainId" - providerAddr := providertypes.NewProviderConsAddress([]byte("providerAddr")) - _, found := providerKeeper.IsValidatorOptedInToChainId(ctx, providerAddr, chainId) - require.False(t, found) - - expectedConsumerId := "consumerId" - providerKeeper.SetConsumerChainId(ctx, expectedConsumerId, chainId) - providerKeeper.SetOptedIn(ctx, expectedConsumerId, providerAddr) - providerKeeper.AppendOptedInConsumerId(ctx, providerAddr, expectedConsumerId) - actualConsumerId, found := providerKeeper.IsValidatorOptedInToChainId(ctx, providerAddr, chainId) - require.True(t, found) - require.Equal(t, expectedConsumerId, actualConsumerId) -} - func TestUpdateAllowlist(t *testing.T) { providerKeeper, ctx, ctrl, _ := testkeeper.GetProviderKeeperAndCtx(t, testkeeper.NewInMemKeeperParams(t)) defer ctrl.Finish() @@ -590,3 +626,97 @@ func TestUpdateMinimumPowerInTopN(t *testing.T) { 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() + + chainId := "chainId" + providerAddr := providertypes.NewProviderConsAddress([]byte("providerAddr")) + _, found := providerKeeper.IsValidatorOptedInToChainId(ctx, providerAddr, chainId) + require.False(t, found) + + expectedConsumerId := "consumerId" + providerKeeper.SetConsumerChainId(ctx, expectedConsumerId, chainId) + providerKeeper.SetOptedIn(ctx, expectedConsumerId, providerAddr) + providerKeeper.AppendOptedInConsumerId(ctx, providerAddr, expectedConsumerId) + actualConsumerId, found := providerKeeper.IsValidatorOptedInToChainId(ctx, providerAddr, chainId) + 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", keeper.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", keeper.Launched) + _, canLaunch = providerKeeper.CanLaunch(ctx, "consumerId") + require.False(t, canLaunch) + + // cannot launch a stopped chain + providerKeeper.SetConsumerPhase(ctx, "consumerId", keeper.Stopped) + _, canLaunch = providerKeeper.CanLaunch(ctx, "consumerId") + require.False(t, canLaunch) + + // initialized chain can launch + providerKeeper.SetConsumerPhase(ctx, "consumerId", keeper.Initialized) + _, canLaunch = providerKeeper.CanLaunch(ctx, "consumerId") + require.True(t, canLaunch) + + // initialized chain that has spawn time in the past cannot launch + ctx = ctx.WithBlockTime(initializationParameters.SpawnTime.Add(time.Second)) + _, canLaunch = providerKeeper.CanLaunch(ctx, "consumerId") + require.False(t, canLaunch) + + // reset block time so the chain can launch + ctx = ctx.WithBlockTime(initializationParameters.SpawnTime.Add(-time.Second)) + _, 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) +} diff --git a/x/ccv/provider/keeper/proposal.go b/x/ccv/provider/keeper/proposal.go index 8361eb0dbb..407d0e2bb3 100644 --- a/x/ccv/provider/keeper/proposal.go +++ b/x/ccv/provider/keeper/proposal.go @@ -357,6 +357,8 @@ func (k Keeper) BeginBlockInit(ctx sdk.Context) { } k.SetConsumerPhase(cachedCtx, consumerId, 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() } } diff --git a/x/ccv/provider/types/errors.go b/x/ccv/provider/types/errors.go index 94e9e5669a..b7ab44d029 100644 --- a/x/ccv/provider/types/errors.go +++ b/x/ccv/provider/types/errors.go @@ -47,4 +47,5 @@ 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") ) From 84565abb1c31fec558f294053013fe6b4dd8c17e Mon Sep 17 00:00:00 2001 From: insumity Date: Tue, 27 Aug 2024 10:15:57 +0200 Subject: [PATCH 3/5] added tests on the hooks --- x/ccv/provider/keeper/distribution.go | 22 ++++---- x/ccv/provider/keeper/hooks.go | 47 ++++------------ x/ccv/provider/keeper/permissionless.go | 52 ++++++++++++++++++ x/ccv/provider/keeper/permissionless_test.go | 58 ++++++++++++++++++++ x/ccv/provider/keeper/proposal.go | 8 +-- 5 files changed, 136 insertions(+), 51 deletions(-) diff --git a/x/ccv/provider/keeper/distribution.go b/x/ccv/provider/keeper/distribution.go index 13698cde32..9291e57a76 100644 --- a/x/ccv/provider/keeper/distribution.go +++ b/x/ccv/provider/keeper/distribution.go @@ -74,33 +74,33 @@ func (k Keeper) AllocateTokens(ctx sdk.Context) { return } - // Iterate over all registered consumer chains - for _, consumerChainID := range k.GetAllRegisteredConsumerIds(ctx) { + // Iterate over all launched consumer chains + for _, consumerId := range k.GetAllRegisteredConsumerIds(ctx) { // note that it's possible that no rewards are collected even though the // reward pool isn't empty. This can happen if the reward pool holds some tokens // of non-whitelisted denominations. - alloc := k.GetConsumerRewardsAllocation(ctx, consumerChainID) + alloc := k.GetConsumerRewardsAllocation(ctx, consumerId) if alloc.Rewards.IsZero() { continue } // temporary workaround to keep CanWithdrawInvariant happy // general discussions here: https://github.com/cosmos/cosmos-sdk/issues/2906#issuecomment-441867634 - if k.ComputeConsumerTotalVotingPower(ctx, consumerChainID) == 0 { + if k.ComputeConsumerTotalVotingPower(ctx, consumerId) == 0 { rewardsToSend, rewardsChange := alloc.Rewards.TruncateDecimal() err := k.distributionKeeper.FundCommunityPool(context.Context(ctx), rewardsToSend, k.accountKeeper.GetModuleAccount(ctx, types.ConsumerRewardsPool).GetAddress()) if err != nil { k.Logger(ctx).Error( "fail to allocate rewards from consumer chain %s to community pool: %s", - consumerChainID, + consumerId, err, ) } // set the consumer allocation to the remaining reward decimals alloc.Rewards = rewardsChange - k.SetConsumerRewardsAllocation(ctx, consumerChainID, alloc) + k.SetConsumerRewardsAllocation(ctx, consumerId, alloc) return } @@ -112,7 +112,7 @@ func (k Keeper) AllocateTokens(ctx sdk.Context) { if err != nil { k.Logger(ctx).Error( "cannot get community tax while allocating rewards from consumer chain %s: %s", - consumerChainID, + consumerId, err, ) continue @@ -132,7 +132,7 @@ func (k Keeper) AllocateTokens(ctx sdk.Context) { if err != nil { k.Logger(ctx).Error( "cannot send rewards to distribution module account %s: %s", - consumerChainID, + consumerId, err, ) continue @@ -141,7 +141,7 @@ func (k Keeper) AllocateTokens(ctx sdk.Context) { // allocate tokens to consumer validators k.AllocateTokensToConsumerValidators( ctx, - consumerChainID, + consumerId, sdk.NewDecCoinsFromCoins(validatorsRewardsTrunc...), ) @@ -151,7 +151,7 @@ func (k Keeper) AllocateTokens(ctx sdk.Context) { if err != nil { k.Logger(ctx).Error( "fail to allocate rewards from consumer chain %s to community pool: %s", - consumerChainID, + consumerId, err, ) continue @@ -159,7 +159,7 @@ func (k Keeper) AllocateTokens(ctx sdk.Context) { // set consumer allocations to the remaining rewards decimals alloc.Rewards = validatorsRewardsChange.Add(remainingChanges...) - k.SetConsumerRewardsAllocation(ctx, consumerChainID, alloc) + k.SetConsumerRewardsAllocation(ctx, consumerId, alloc) } } diff --git a/x/ccv/provider/keeper/hooks.go b/x/ccv/provider/keeper/hooks.go index 841db8330e..a1576ff33d 100644 --- a/x/ccv/provider/keeper/hooks.go +++ b/x/ccv/provider/keeper/hooks.go @@ -115,44 +115,19 @@ func (h Hooks) AfterProposalSubmission(goCtx context.Context, proposalId uint64) return fmt.Errorf("cannot retrieve proposal with id: %d", proposalId) } - hasUpdateConsumerMsg := false - for _, msg := range p.GetMessages() { - sdkMsg, isMsgUpdateConsumer := msg.GetCachedValue().(*providertypes.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 := h.k.GetConsumerOwnerAddress(ctx, sdkMsg.ConsumerId) - if err != nil { - return fmt.Errorf("cannot find owner address for consumer with consumer id (%s): %s", sdkMsg.ConsumerId, err.Error()) - } else if ownerAddress != h.k.GetAuthority() { - return fmt.Errorf("owner address (%s) is not the gov module (%s)", ownerAddress, h.k.GetAuthority()) - } - - if hasUpdateConsumerMsg { - return fmt.Errorf("proposal can contain at most one `MsgUpdateConsumer` message") - } - hasUpdateConsumerMsg = true - h.k.SetProposalIdToConsumerId(ctx, proposalId, sdkMsg.ConsumerId) - } + err = DoesNotHaveDeprecatedMessage(&p) + if err != nil { + return err + } - // if the proposal contains a deprecated message, cancel the proposal - _, isMsgConsumerAddition := msg.GetCachedValue().(*providertypes.MsgConsumerAddition) - if isMsgConsumerAddition { - return fmt.Errorf("proposal cannot contain deprecated `MsgConsumerAddition`; use `MsgCreateConsumer` instead") - } + msgUpdateConsumer, err := h.k.HasAtMostOnceCorrectMsgUpdateConsumer(ctx, &p) + if err != nil { + return err + } - _, isMsgConsumerModification := msg.GetCachedValue().(*providertypes.MsgConsumerModification) - if isMsgConsumerModification { - return fmt.Errorf("proposal cannot contain deprecated `MsgConsumerModification`; use `MsgUpdateConsumer` instead") - } - _, isMsgConsumerRemoval := msg.GetCachedValue().(*providertypes.MsgConsumerRemoval) - if isMsgConsumerRemoval { - return fmt.Errorf("proposal cannot contain deprecated `MsgConsumerRemoval`; use `MsgRemoveConsumer` instead") - } + if msgUpdateConsumer != nil { + // a correctly set `MsgUpdateConsumer` was found + h.k.SetProposalIdToConsumerId(ctx, proposalId, msgUpdateConsumer.ConsumerId) } return nil diff --git a/x/ccv/provider/keeper/permissionless.go b/x/ccv/provider/keeper/permissionless.go index 1c4a6a5597..af979ff07e 100644 --- a/x/ccv/provider/keeper/permissionless.go +++ b/x/ccv/provider/keeper/permissionless.go @@ -6,6 +6,7 @@ import ( "encoding/binary" "fmt" 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" "strconv" "time" @@ -692,3 +693,54 @@ func (k Keeper) CanLaunch(ctx sdk.Context, consumerId string) (time.Time, bool) return initializationParameters.SpawnTime, spawnTimeInTheFuture && 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 7e290dea5b..ebd02e61d4 100644 --- a/x/ccv/provider/keeper/permissionless_test.go +++ b/x/ccv/provider/keeper/permissionless_test.go @@ -2,6 +2,7 @@ package keeper_test 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" @@ -720,3 +721,60 @@ func TestCanLaunch(t *testing.T) { 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) + 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) +} diff --git a/x/ccv/provider/keeper/proposal.go b/x/ccv/provider/keeper/proposal.go index 407d0e2bb3..e16ec3ebff 100644 --- a/x/ccv/provider/keeper/proposal.go +++ b/x/ccv/provider/keeper/proposal.go @@ -488,13 +488,13 @@ func (k Keeper) BeginBlockCCR(ctx sdk.Context) { "err", err.Error()) 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()) k.SetConsumerPhase(cachedCtx, consumerId, Stopped) k.RemoveConsumerToBeStoppedFromStopTime(ctx, consumerId, stopTime) + + // 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", From 0fc69685c98dd9c1aa7ebbeb017e6a564fa07335 Mon Sep 17 00:00:00 2001 From: insumity Date: Tue, 27 Aug 2024 13:04:29 +0200 Subject: [PATCH 4/5] removed check that spawn time is in the future --- x/ccv/provider/keeper/permissionless.go | 5 ++--- x/ccv/provider/keeper/permissionless_test.go | 10 ---------- 2 files changed, 2 insertions(+), 13 deletions(-) diff --git a/x/ccv/provider/keeper/permissionless.go b/x/ccv/provider/keeper/permissionless.go index af979ff07e..7353b13f86 100644 --- a/x/ccv/provider/keeper/permissionless.go +++ b/x/ccv/provider/keeper/permissionless.go @@ -680,8 +680,7 @@ func (k Keeper) CanLaunch(ctx sdk.Context, consumerId string) (time.Time, bool) return time.Time{}, false } - // a chain can only launch if the spawn time is in the future - spawnTimeInTheFuture := initializationParameters.SpawnTime.After(ctx.BlockTime()) + spawnTimeIsNotZero := !initializationParameters.SpawnTime.Equal(time.Time{}) genesisHashSet := initializationParameters.GenesisHash != nil binaryHashSet := initializationParameters.BinaryHash != nil @@ -690,7 +689,7 @@ func (k Keeper) CanLaunch(ctx sdk.Context, consumerId string) (time.Time, bool) blocksPerDistributionTransmissionSet := initializationParameters.BlocksPerDistributionTransmission > 0 historicalEntriesSet := initializationParameters.HistoricalEntries > 0 - return initializationParameters.SpawnTime, spawnTimeInTheFuture && genesisHashSet && binaryHashSet && consumerRedistributionFractionSet && + return initializationParameters.SpawnTime, spawnTimeIsNotZero && genesisHashSet && binaryHashSet && consumerRedistributionFractionSet && blocksPerDistributionTransmissionSet && historicalEntriesSet } diff --git a/x/ccv/provider/keeper/permissionless_test.go b/x/ccv/provider/keeper/permissionless_test.go index ebd02e61d4..a18fe738cc 100644 --- a/x/ccv/provider/keeper/permissionless_test.go +++ b/x/ccv/provider/keeper/permissionless_test.go @@ -704,16 +704,6 @@ func TestCanLaunch(t *testing.T) { _, canLaunch = providerKeeper.CanLaunch(ctx, "consumerId") require.True(t, canLaunch) - // initialized chain that has spawn time in the past cannot launch - ctx = ctx.WithBlockTime(initializationParameters.SpawnTime.Add(time.Second)) - _, canLaunch = providerKeeper.CanLaunch(ctx, "consumerId") - require.False(t, canLaunch) - - // reset block time so the chain can launch - ctx = ctx.WithBlockTime(initializationParameters.SpawnTime.Add(-time.Second)) - _, 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) From 19be098404dc1a41ff46b961d2f283215e95eec9 Mon Sep 17 00:00:00 2001 From: Simon Noetzlin Date: Tue, 27 Aug 2024 17:06:11 +0200 Subject: [PATCH 5/5] feat: refactor consumer validator set computation (#2175) * add UT * nits * address comments * Update x/ccv/provider/keeper/partial_set_security.go Co-authored-by: insumity * fix tests --------- Co-authored-by: insumity --- x/ccv/provider/keeper/grpc_query.go | 36 +++--- x/ccv/provider/keeper/partial_set_security.go | 57 ++++++++- .../keeper/partial_set_security_test.go | 113 ++++++++++++++++-- x/ccv/provider/keeper/proposal.go | 6 +- x/ccv/provider/keeper/relay.go | 5 +- 5 files changed, 177 insertions(+), 40 deletions(-) diff --git a/x/ccv/provider/keeper/grpc_query.go b/x/ccv/provider/keeper/grpc_query.go index 5408cc606a..4a167599ec 100644 --- a/x/ccv/provider/keeper/grpc_query.go +++ b/x/ccv/provider/keeper/grpc_query.go @@ -437,29 +437,29 @@ func (k Keeper) hasToValidate( if err != nil { return false, nil } + + minPowerToOptIn := int64(0) + // If the consumer is TopN compute the minimum power if topN := k.GetTopN(ctx, consumerId); topN > 0 { - // in a Top-N chain, we automatically opt in all validators that belong to the top N - minPower, found := k.GetMinimumPowerInTopN(ctx, consumerId) - if found { - k.OptInTopNValidators(ctx, consumerId, activeValidators, minPower) - } else { - k.Logger(ctx).Error("did not find min power in top N for chain", "chain", consumerId) + // compute the minimum power to opt-in since the one in the state is stale + // Note that the effective min power will be computed at the end of the epoch + minPowerToOptIn, err = k.ComputeMinPowerInTopN(ctx, activeValidators, topN) + if err != nil { + return false, err } } - // if the validator is opted in and belongs to the validators of the next epoch, then if nothing changes + // if the validator belongs to the validators of the next epoch, then if nothing changes // the validator would have to validate in the next epoch - if k.IsOptedIn(ctx, consumerId, provAddr) { - lastVals, err := k.GetLastBondedValidators(ctx) - if err != nil { - return false, err - } - nextValidators := k.ComputeNextValidators(ctx, consumerId, lastVals) - for _, v := range nextValidators { - consAddr := sdk.ConsAddress(v.ProviderConsAddr) - if provAddr.ToSdkConsAddr().Equals(consAddr) { - return true, nil - } + lastVals, err := k.GetLastBondedValidators(ctx) + if err != nil { + return false, err + } + nextValidators := k.ComputeNextValidators(ctx, consumerId, lastVals, minPowerToOptIn) + for _, v := range nextValidators { + consAddr := sdk.ConsAddress(v.ProviderConsAddr) + if provAddr.ToSdkConsAddr().Equals(consAddr) { + return true, nil } } diff --git a/x/ccv/provider/keeper/partial_set_security.go b/x/ccv/provider/keeper/partial_set_security.go index 305f0f358b..6787429a6f 100644 --- a/x/ccv/provider/keeper/partial_set_security.go +++ b/x/ccv/provider/keeper/partial_set_security.go @@ -137,7 +137,6 @@ func (k Keeper) OptInTopNValidators(ctx sdk.Context, consumerId string, bondedVa // if validator already exists it gets overwritten k.SetOptedIn(ctx, consumerId, types.NewProviderConsAddress(consAddr)) - k.SetOptedIn(ctx, consumerId, types.NewProviderConsAddress(consAddr)) } // else validators that do not belong to the top N validators but were opted in, remain opted in } } @@ -322,9 +321,17 @@ func NoMoreThanPercentOfTheSum(validators []types.ConsensusValidator, percent ui // CanValidateChain returns true if the validator `providerAddr` is opted-in to chain with `consumerId` and the allowlist // and denylist do not prevent the validator from validating the chain. -func (k Keeper) CanValidateChain(ctx sdk.Context, consumerId string, providerAddr types.ProviderConsAddress) bool { +func (k Keeper) CanValidateChain(ctx sdk.Context, consumerId string, providerAddr types.ProviderConsAddress, minPowerToOptIn int64) bool { + // check if the validator is already opted-in + optedIn := k.IsOptedIn(ctx, consumerId, providerAddr) + + // check if the validator is automatically opted-in for a topN chain + if !optedIn && k.GetTopN(ctx, consumerId) > 0 { + optedIn = k.HasMinPower(ctx, providerAddr, minPowerToOptIn) + } + // only consider opted-in validators - return k.IsOptedIn(ctx, consumerId, providerAddr) && + return optedIn && // if an allowlist is declared, only consider allowlisted validators (k.IsAllowlistEmpty(ctx, consumerId) || k.IsAllowlisted(ctx, consumerId, providerAddr)) && @@ -352,7 +359,7 @@ func (k Keeper) FulfillsMinStake(ctx sdk.Context, consumerId string, providerAdd } // ComputeNextValidators computes the validators for the upcoming epoch based on the currently `bondedValidators`. -func (k Keeper) ComputeNextValidators(ctx sdk.Context, consumerId string, bondedValidators []stakingtypes.Validator) []types.ConsensusValidator { +func (k Keeper) ComputeNextValidators(ctx sdk.Context, consumerId string, bondedValidators []stakingtypes.Validator, minPowerToOptIn int64) []types.ConsensusValidator { // sort the bonded validators by number of staked tokens in descending order sort.Slice(bondedValidators, func(i, j int) bool { return bondedValidators[i].GetBondedTokens().GT(bondedValidators[j].GetBondedTokens()) @@ -371,9 +378,49 @@ func (k Keeper) ComputeNextValidators(ctx sdk.Context, consumerId string, bonded nextValidators := k.FilterValidators(ctx, consumerId, bondedValidators, func(providerAddr types.ProviderConsAddress) bool { - return k.CanValidateChain(ctx, consumerId, providerAddr) && k.FulfillsMinStake(ctx, consumerId, providerAddr) + return k.CanValidateChain(ctx, consumerId, providerAddr, minPowerToOptIn) && k.FulfillsMinStake(ctx, consumerId, providerAddr) }) nextValidators = k.CapValidatorSet(ctx, consumerId, nextValidators) return k.CapValidatorsPower(ctx, consumerId, nextValidators) } + +// HasMinPower returns true if the `providerAddr` voting power is GTE than the given minimum power +func (k Keeper) HasMinPower(ctx sdk.Context, providerAddr types.ProviderConsAddress, minPower int64) bool { + val, err := k.stakingKeeper.GetValidatorByConsAddr(ctx, providerAddr.Address) + if err != nil { + k.Logger(ctx).Error( + "cannot get last validator power", + "provider address", + providerAddr, + "error", + err, + ) + return false + } + + valAddr, err := sdk.ValAddressFromBech32(val.GetOperator()) + if err != nil { + k.Logger(ctx).Error( + "could not retrieve validator address", + "operator address", + val.GetOperator(), + "error", + err, + ) + return false + } + + power, err := k.stakingKeeper.GetLastValidatorPower(ctx, valAddr) + if err != nil { + k.Logger(ctx).Error("could not retrieve last power of validator address", + "operator address", + val.GetOperator(), + "error", + err, + ) + return false + } + + return power >= minPower +} diff --git a/x/ccv/provider/keeper/partial_set_security_test.go b/x/ccv/provider/keeper/partial_set_security_test.go index c710e7f299..671150f865 100644 --- a/x/ccv/provider/keeper/partial_set_security_test.go +++ b/x/ccv/provider/keeper/partial_set_security_test.go @@ -2,6 +2,7 @@ package keeper_test import ( "bytes" + "fmt" "sort" "testing" @@ -384,29 +385,45 @@ func TestCanValidateChain(t *testing.T) { providerKeeper, ctx, ctrl, mocks := testkeeper.GetProviderKeeperAndCtx(t, testkeeper.NewInMemKeeperParams(t)) defer ctrl.Finish() - validator := createStakingValidator(ctx, mocks, 0, 1, 1) + consumerID := "0" + + validator := createStakingValidator(ctx, mocks, 0, 1, 1) // adds GetLastValidatorPower expectation to mocks consAddr, _ := validator.GetConsAddr() providerAddr := types.NewProviderConsAddress(consAddr) // with no allowlist or denylist, the validator has to be opted in, in order to consider it - require.False(t, providerKeeper.CanValidateChain(ctx, "consumerId", providerAddr)) - providerKeeper.SetOptedIn(ctx, "consumerId", types.NewProviderConsAddress(consAddr)) - require.True(t, providerKeeper.CanValidateChain(ctx, "consumerId", providerAddr)) + require.False(t, providerKeeper.CanValidateChain(ctx, consumerID, providerAddr, 0)) + + // with TopN chains, the validator can be considered, + mocks.MockStakingKeeper.EXPECT().GetValidatorByConsAddr(gomock.Any(), providerAddr.Address).Return(validator, nil).Times(2) + providerKeeper.SetConsumerPowerShapingParameters(ctx, consumerID, types.PowerShapingParameters{Top_N: 50}) + // validator's power is LT the min power + require.False(t, providerKeeper.CanValidateChain(ctx, consumerID, providerAddr, 2)) + // validator's power is GTE the min power + require.True(t, providerKeeper.CanValidateChain(ctx, consumerID, providerAddr, 1)) + + // when validator is opted-in it can validate regardless of its min power + providerKeeper.SetOptedIn(ctx, consumerID, types.NewProviderConsAddress(consAddr)) + require.True(t, providerKeeper.CanValidateChain(ctx, consumerID, providerAddr, 2)) + + // With OptIn chains, validator can validate only if it has already opted-in + providerKeeper.SetConsumerPowerShapingParameters(ctx, consumerID, types.PowerShapingParameters{Top_N: 0}) + require.True(t, providerKeeper.CanValidateChain(ctx, consumerID, providerAddr, 2)) // create an allow list but do not add the validator `providerAddr` to it validatorA := createStakingValidator(ctx, mocks, 1, 1, 2) consAddrA, _ := validatorA.GetConsAddr() - providerKeeper.SetAllowlist(ctx, "consumerId", types.NewProviderConsAddress(consAddrA)) - require.False(t, providerKeeper.CanValidateChain(ctx, "consumerId", providerAddr)) - providerKeeper.SetAllowlist(ctx, "consumerId", types.NewProviderConsAddress(consAddr)) - require.True(t, providerKeeper.CanValidateChain(ctx, "consumerId", providerAddr)) + providerKeeper.SetAllowlist(ctx, consumerID, types.NewProviderConsAddress(consAddrA)) + require.False(t, providerKeeper.CanValidateChain(ctx, consumerID, providerAddr, 1)) + providerKeeper.SetAllowlist(ctx, consumerID, types.NewProviderConsAddress(consAddr)) + require.True(t, providerKeeper.CanValidateChain(ctx, consumerID, providerAddr, 1)) // create a denylist but do not add validator `providerAddr` to it - providerKeeper.SetDenylist(ctx, "consumerId", types.NewProviderConsAddress(consAddrA)) - require.True(t, providerKeeper.CanValidateChain(ctx, "consumerId", providerAddr)) + providerKeeper.SetDenylist(ctx, consumerID, types.NewProviderConsAddress(consAddrA)) + require.True(t, providerKeeper.CanValidateChain(ctx, consumerID, providerAddr, 1)) // add validator `providerAddr` to the denylist - providerKeeper.SetDenylist(ctx, "consumerId", types.NewProviderConsAddress(consAddr)) - require.False(t, providerKeeper.CanValidateChain(ctx, "consumerId", providerAddr)) + providerKeeper.SetDenylist(ctx, consumerID, types.NewProviderConsAddress(consAddr)) + require.False(t, providerKeeper.CanValidateChain(ctx, consumerID, providerAddr, 1)) } func TestCapValidatorSet(t *testing.T) { @@ -820,7 +837,7 @@ func TestIfInactiveValsDisallowedProperty(t *testing.T) { providerKeeper.SetParams(ctx, params) // Compute the next validators - nextVals := providerKeeper.ComputeNextValidators(ctx, "consumerId", vals) + nextVals := providerKeeper.ComputeNextValidators(ctx, "consumerId", vals, 0) // Check that the length of nextVals is at most maxProviderConsensusVals require.LessOrEqual(r, len(nextVals), int(maxProviderConsensusVals), "The length of nextVals should be at most maxProviderConsensusVals") @@ -843,3 +860,73 @@ func TestIfInactiveValsDisallowedProperty(t *testing.T) { } }) } + +func TestHasMinPower(t *testing.T) { + pk, ctx, ctrl, mocks := testkeeper.GetProviderKeeperAndCtx(t, testkeeper.NewInMemKeeperParams(t)) + defer ctrl.Finish() + + providerConsPubKey := ed25519.GenPrivKeyFromSecret([]byte{1}).PubKey() + consAddr := sdk.ConsAddress(providerConsPubKey.Address()) + providerAddr := types.NewProviderConsAddress(consAddr) + + testCases := []struct { + name string + minPower uint64 + expectation func(sdk.ConsAddress, testkeeper.MockedKeepers) + hasMinPower bool + }{ + { + name: "cannot find validator by cons address", + expectation: func(sdk.ConsAddress, testkeeper.MockedKeepers) { + mocks.MockStakingKeeper.EXPECT().GetValidatorByConsAddr(gomock.Any(), consAddr). + Return(stakingtypes.Validator{}, fmt.Errorf("validator not found")).Times(1) + }, + hasMinPower: false, + }, { + name: "cannot convert validator operator address", + expectation: func(consAddr sdk.ConsAddress, mocks testkeeper.MockedKeepers) { + mocks.MockStakingKeeper.EXPECT().GetValidatorByConsAddr(gomock.Any(), consAddr).Return(stakingtypes.Validator{OperatorAddress: "xxxx"}, nil).Times(1) + }, + hasMinPower: false, + }, { + name: "cannot find last validator power", + expectation: func(consAddr sdk.ConsAddress, mocks testkeeper.MockedKeepers) { + valAddr := sdk.ValAddress(providerAddr.Address.Bytes()) + mocks.MockStakingKeeper.EXPECT().GetValidatorByConsAddr(gomock.Any(), consAddr). + Return(stakingtypes.Validator{OperatorAddress: valAddr.String()}, nil) + mocks.MockStakingKeeper.EXPECT().GetLastValidatorPower(gomock.Any(), valAddr). + Return(int64(0), fmt.Errorf("last power not found")).Times(1) + }, + hasMinPower: false, + }, { + name: "validator power is LT min power", + expectation: func(consAddr sdk.ConsAddress, mocks testkeeper.MockedKeepers) { + valAddr := sdk.ValAddress(providerAddr.Address.Bytes()) + mocks.MockStakingKeeper.EXPECT().GetValidatorByConsAddr(gomock.Any(), consAddr). + Return(stakingtypes.Validator{OperatorAddress: valAddr.String()}, nil) + mocks.MockStakingKeeper.EXPECT().GetLastValidatorPower(gomock.Any(), valAddr). + Return(int64(5), nil).Times(1) + }, + hasMinPower: false, + }, { + name: "validator power is GTE min power", + expectation: func(consAddr sdk.ConsAddress, mocks testkeeper.MockedKeepers) { + valAddr := sdk.ValAddress(providerAddr.Address.Bytes()) + mocks.MockStakingKeeper.EXPECT().GetValidatorByConsAddr(gomock.Any(), consAddr). + Return(stakingtypes.Validator{OperatorAddress: valAddr.String()}, nil) + mocks.MockStakingKeeper.EXPECT().GetLastValidatorPower(gomock.Any(), valAddr). + Return(int64(10), nil).Times(1) + }, + hasMinPower: true, + }, + } + + minPower := int64(10) + + for _, tc := range testCases { + t.Run(tc.name, func(t *testing.T) { + tc.expectation(consAddr, mocks) + require.Equal(t, tc.hasMinPower, pk.HasMinPower(ctx, providerAddr, minPower)) + }) + } +} diff --git a/x/ccv/provider/keeper/proposal.go b/x/ccv/provider/keeper/proposal.go index e16ec3ebff..fb1920c453 100644 --- a/x/ccv/provider/keeper/proposal.go +++ b/x/ccv/provider/keeper/proposal.go @@ -232,6 +232,7 @@ func (k Keeper) MakeConsumerGenesis( 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 @@ -243,7 +244,7 @@ func (k Keeper) MakeConsumerGenesis( } // 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) + minPower, err = k.ComputeMinPowerInTopN(ctx, activeValidators, powerShapingParameters.Top_N) if err != nil { return gen, nil, err } @@ -255,8 +256,9 @@ func (k Keeper) MakeConsumerGenesis( 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) + nextValidators := k.ComputeNextValidators(ctx, consumerId, bondedValidators, minPower) k.SetConsumerValSet(ctx, consumerId, nextValidators) // get the initial updates with the latest set consumer public keys diff --git a/x/ccv/provider/keeper/relay.go b/x/ccv/provider/keeper/relay.go index b0e8bb4e83..cfa8ec5a9f 100644 --- a/x/ccv/provider/keeper/relay.go +++ b/x/ccv/provider/keeper/relay.go @@ -199,6 +199,7 @@ func (k Keeper) QueueVSCPackets(ctx sdk.Context) { } topN := k.GetTopN(ctx, consumerId) + minPower := int64(0) if topN > 0 { // in a Top-N chain, we automatically opt in all validators that belong to the top N // of the active validators @@ -208,7 +209,7 @@ func (k Keeper) QueueVSCPackets(ctx sdk.Context) { panic(fmt.Errorf("failed to get active validators: %w", err)) } - minPower, err := k.ComputeMinPowerInTopN(ctx, activeValidators, topN) + minPower, err = k.ComputeMinPowerInTopN(ctx, activeValidators, topN) if err != nil { // we panic, since the only way to proceed would be to opt in all validators, which is not the intended behavior panic(fmt.Errorf("failed to compute min power to opt in for chain %v: %w", consumerId, err)) @@ -220,7 +221,7 @@ func (k Keeper) QueueVSCPackets(ctx sdk.Context) { k.OptInTopNValidators(ctx, consumerId, activeValidators, minPower) } - nextValidators := k.ComputeNextValidators(ctx, consumerId, bondedValidators) + nextValidators := k.ComputeNextValidators(ctx, consumerId, bondedValidators, minPower) valUpdates := DiffValidators(currentValidators, nextValidators) k.SetConsumerValSet(ctx, consumerId, nextValidators)