Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat!: only allow chains with at least one active validator to launch #2399

Merged
merged 3 commits into from
Dec 3, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
- `[x/provider]` Prevent Opt-In chains from launching, unless at least one active validator has opted-in to them.
([\#2101](https://github.com/cosmos/interchain-security/pull/2399))
1 change: 1 addition & 0 deletions tests/interchain/chainsuite/chain_spec_provider.go
Original file line number Diff line number Diff line change
Expand Up @@ -51,5 +51,6 @@ func providerModifiedGenesis() []cosmos.GenesisKV {
cosmos.NewGenesisKV("app_state.provider.params.slash_meter_replenish_period", ProviderReplenishPeriod),
cosmos.NewGenesisKV("app_state.provider.params.slash_meter_replenish_fraction", ProviderReplenishFraction),
cosmos.NewGenesisKV("app_state.provider.params.blocks_per_epoch", "1"),
cosmos.NewGenesisKV("app_state.staking.params.max_validators", "1"),
}
}
19 changes: 11 additions & 8 deletions tests/interchain/chainsuite/config.go
Original file line number Diff line number Diff line change
Expand Up @@ -32,12 +32,14 @@ const (
CommitTimeout = 2 * time.Second
TotalValidatorFunds = 11_000_000_000
ValidatorFunds = 30_000_000
ValidatorCount = 1
FullNodeCount = 0
ChainSpawnWait = 155 * time.Second
CosmosChainType = "cosmos"
GovModuleAddress = "cosmos10d07y265gmmuvt4z0w9aw880jnsr700j6zn9kn"
TestWalletsNumber = 15 // Ensure that test accounts are used in a way that maintains the mutual independence of tests
// ValidatorCount is set to 2, so we have one active and one inactive (i.e., outside the active set) validator.
// Note that the provider has at most 1 validator (see `chain_spec_provider.go`).
ValidatorCount = 2
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is this done for al the tests or only for the new one introduced? We should avoid adding complexity to all tests. If you need multiple validators for a certain test, we should just add a new configuration.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This will affect all tests in the suite that use this specification, i.e., the entire ProviderSuite. We could create a separate suite to support running tests on a suite with multiple nodes while keeping the existing suite where the provider chain has only one node (ProviderSuite, ProviderMultiNodeSuite).
Another option is to keep it as Karolos implemented and handle the optional nodes in the suite setup, so tests that need additional nodes can simply start the extra validator(s) as the interchaintest framework supports that.
The difference is that with separate suites, we don’t need to start/stop nodes dynamically, but the provider chain will be started twice. I'm not sure what the optimal solution would be.
If you'd like, we can leave it as Karolos implemented in this PR, and I can split it or add dynamic node management in a follow-up PR. I also plan to improve how "independent" test accounts are fetched, so I can handle everything in that PR.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sounds good. Let's do that.
Regarding what option to choose, go with the one easier to implement.

FullNodeCount = 0
ChainSpawnWait = 155 * time.Second
CosmosChainType = "cosmos"
GovModuleAddress = "cosmos10d07y265gmmuvt4z0w9aw880jnsr700j6zn9kn"
TestWalletsNumber = 15 // Ensure that test accounts are used in a way that maintains the mutual independence of tests
)

func DefaultConfigToml() testutil.Toml {
Expand All @@ -51,13 +53,14 @@ func DefaultConfigToml() testutil.Toml {
}

func DefaultGenesisAmounts(denom string) func(i int) (sdktypes.Coin, sdktypes.Coin) {
// Returns an amount of funds per validator, so validator with val index 0 has the most funds, then validator 1, then validator 2, etc.
return func(i int) (sdktypes.Coin, sdktypes.Coin) {
return sdktypes.Coin{
Denom: denom,
Amount: sdkmath.NewInt(TotalValidatorFunds),
Amount: sdkmath.NewInt(TotalValidatorFunds / int64(i+1)),
}, sdktypes.Coin{
Denom: denom,
Amount: sdkmath.NewInt(ValidatorFunds),
Amount: sdkmath.NewInt(ValidatorFunds / int64(i+1)),
}
}
}
52 changes: 49 additions & 3 deletions tests/interchain/provider_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,7 @@ func (s *ProviderSuite) TestProviderCreateConsumer() {
testAcc := s.Provider.TestWallets[0].FormattedAddress()
testAccKey := s.Provider.TestWallets[0].KeyName()

// Confirm that a chain can be create with the minimum params (metadata)
// Confirm that a chain can be created with the minimum params (metadata)
chainName := "minParamAddConsumer"
createConsumerMsg := msgCreateConsumer(chainName, nil, nil, testAcc)
consumerId, err := s.Provider.CreateConsumer(s.GetContext(), createConsumerMsg, testAccKey)
Expand Down Expand Up @@ -101,6 +101,52 @@ func (s *ProviderSuite) TestProviderCreateConsumerRejection() {
s.Require().Error(err)
}

// TestOptInChainCanOnlyStartIfActiveValidatorOptedIn tests that only if an active validator opts in to an Opt-In chain, the chain can launch.
// Scenario 1: Inactive validators opts in, the chain does not launch.
// Scenario 2: Active validator opts in, the chain launches.
func (s *ProviderSuite) TestOptInChainCanOnlyStartIfActiveValidatorOptedIn() {
testAcc := s.Provider.TestWallets[2].FormattedAddress()
Copy link
Collaborator

@stana-miric stana-miric Dec 3, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'll create a separate PR so that we don't have to be aware of which index is in use during the tests, and it will be handled under the hood. For now, I would use an account that isn't used in other tests, for example, account s.Provider.TestWallets[12] (just to avoid possible an error where an invalid account number is sent if 2 tests send the tx in the same time). I'll try to fix this as soon as possible since the current solution isn't the most elegant.

testAccKey := s.Provider.TestWallets[2].KeyName()

activeValIndex := 0
inactiveValIndex := 1

// Scenario 1: Inactive validators opts in, the chain does not launch.
chainName := "optInScenario1"
spawnTime := time.Now().Add(time.Hour)
consumerInitParams := consumerInitParamsTemplate(&spawnTime)
createConsumerMsg := msgCreateConsumer(chainName, consumerInitParams, powerShapingParamsTemplate(), testAcc)
consumerId, err := s.Provider.CreateConsumer(s.GetContext(), createConsumerMsg, testAccKey)
s.Require().NoError(err)
consumerChain, err := s.Provider.GetConsumerChain(s.GetContext(), consumerId)
s.Require().NoError(err)
// inactive validator opts in
s.Require().NoError(s.Provider.OptIn(s.GetContext(), consumerChain.ConsumerID, inactiveValIndex))
consumerInitParams.SpawnTime = time.Now()
upgradeMsg := &providertypes.MsgUpdateConsumer{
Owner: testAcc,
ConsumerId: consumerChain.ConsumerID,
NewOwnerAddress: testAcc,
InitializationParameters: consumerInitParams,
PowerShapingParameters: powerShapingParamsTemplate(),
}
s.Require().NoError(s.Provider.UpdateConsumer(s.GetContext(), upgradeMsg, testAccKey))
s.Require().NoError(testutil.WaitForBlocks(s.GetContext(), 1, s.Provider))
consumerChain, err = s.Provider.GetConsumerChain(s.GetContext(), consumerId)
s.Require().NoError(err)
s.Require().Equal(providertypes.CONSUMER_PHASE_REGISTERED.String(), consumerChain.Phase)

// Scenario 2: Active validator opts in, the chain launches.
// active validator opts in
s.Require().NoError(s.Provider.OptIn(s.GetContext(), consumerChain.ConsumerID, activeValIndex))

s.Require().NoError(s.Provider.UpdateConsumer(s.GetContext(), upgradeMsg, testAccKey))
s.Require().NoError(testutil.WaitForBlocks(s.GetContext(), 1, s.Provider))
consumerChain, err = s.Provider.GetConsumerChain(s.GetContext(), consumerId)
s.Require().NoError(err)
s.Require().Equal(providertypes.CONSUMER_PHASE_LAUNCHED.String(), consumerChain.Phase)
}

// Test Opting in validators to a chain (MsgOptIn)
// Confirm that a chain can be created and validators can be opted in
// Scenario 1: Validators opted in, MsgUpdateConsumer called to set spawn time in the past -> chain should start.
Expand Down Expand Up @@ -403,12 +449,12 @@ func (s *ProviderSuite) TestProviderTransformTopNtoOptIn() {
s.Require().Equal(testAcc, optInChain.OwnerAddress)
}

// TestOptOut tests removin validator from consumer-opted-in-validators
// TestOptOut tests removing validator from consumer-opted-in-validators
func (s *ProviderSuite) TestOptOut() {
testAcc := s.Provider.TestWallets[7].FormattedAddress()
testAccKey := s.Provider.TestWallets[7].KeyName()

// Add consume chain
// Add consumer chain
chainName := "TestOptOut"
spawnTime := time.Now().Add(time.Hour)
consumerInitParams := consumerInitParamsTemplate(&spawnTime)
Expand Down
38 changes: 37 additions & 1 deletion x/ccv/provider/keeper/consumer_lifecycle.go
Original file line number Diff line number Diff line change
Expand Up @@ -190,6 +190,33 @@ func (k Keeper) ConsumeIdsFromTimeQueue(
return result, nil
}

// HasActiveConsumerValidator checks whether at least one active validator is opted in to chain with `consumerId`
func (k Keeper) HasActiveConsumerValidator(ctx sdk.Context, consumerId string, activeValidators []stakingtypes.Validator) (bool, error) {
currentValidatorSet, err := k.GetConsumerValSet(ctx, consumerId)
if err != nil {
return false, fmt.Errorf("getting consumer validator set of chain with consumerId (%s): %w", consumerId, err)
}

isActiveValidator := make(map[string]bool)
for _, val := range activeValidators {
consAddr, err := val.GetConsAddr()
if err != nil {
return false, fmt.Errorf("getting consensus address of validator (%+v), consumerId (%s): %w", val, consumerId, err)
}
providerConsAddr := types.NewProviderConsAddress(consAddr)
isActiveValidator[providerConsAddr.String()] = true
}

for _, val := range currentValidatorSet {
providerConsAddr := types.NewProviderConsAddress(val.ProviderConsAddr)
if isActiveValidator[providerConsAddr.String()] {
return true, nil
}
}

return false, nil
}

// LaunchConsumer launches the chain with the provided consumer id by creating the consumer client and the respective
// consumer genesis file
//
Expand All @@ -205,8 +232,17 @@ func (k Keeper) LaunchConsumer(
if err != nil {
return fmt.Errorf("computing consumer next validator set, consumerId(%s): %w", consumerId, err)
}

if len(initialValUpdates) == 0 {
return fmt.Errorf("cannot launch consumer with no validator opted in, consumerId(%s)", consumerId)
return fmt.Errorf("cannot launch consumer with no consumer validator, consumerId(%s)", consumerId)
}

hasActiveConsumerValidator, err := k.HasActiveConsumerValidator(ctx, consumerId, activeValidators)
if err != nil {
return fmt.Errorf("cannot check if chain has an active consumer validator, consumerId(%s): %w", consumerId, err)
}
if !hasActiveConsumerValidator {
return fmt.Errorf("cannot launch consumer with no active consumer validator, consumerId(%s)", consumerId)
}

// create consumer genesis
Expand Down
61 changes: 61 additions & 0 deletions x/ccv/provider/keeper/consumer_lifecycle_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -472,6 +472,67 @@ func TestConsumeIdsFromTimeQueue(t *testing.T) {
}
}

func TestHasActiveValidatorOptedIn(t *testing.T) {
keeperParams := testkeeper.NewInMemKeeperParams(t)
providerKeeper, ctx, _, mocks := testkeeper.GetProviderKeeperAndCtx(t, keeperParams)

// set 5 bonded validators with powers 5, 4, 3, 2, and 1
NumberOfBondedValidators := 5
var bondedValidators []stakingtypes.Validator
for i := 0; i < NumberOfBondedValidators; i++ {
power := int64(NumberOfBondedValidators - i)
bondedValidators = append(bondedValidators, createStakingValidator(ctx, mocks, power, i))
}
mocks.MockStakingKeeper.EXPECT().GetBondedValidatorsByPower(gomock.Any()).Return(bondedValidators, nil).AnyTimes()

// get the consensus addresses of the previously-set bonded validators
var consensusAddresses [][]byte
for i := 0; i < NumberOfBondedValidators; i++ {
consAddr, _ := bondedValidators[i].GetConsAddr()
consensusAddresses = append(consensusAddresses, consAddr)
}

// Set the maximum number of provider consensus active validators (i.e., active validators) to 3. As a result
// `bondedValidators[0]` (with power of 5), `bondedValidators[1]` (with power of 4), `bondedValidators[2]` (with power of 3)
// are the active validators, and `bondedValidators[3]` (with power of 2) and `bondedValidators[4]` (with power of 1)
// are non-active validators.
maxProviderConsensusValidators := int64(3)
params := providerKeeper.GetParams(ctx)
params.MaxProviderConsensusValidators = maxProviderConsensusValidators
providerKeeper.SetParams(ctx, params)

activeValidators, _ := providerKeeper.GetLastProviderConsensusActiveValidators(ctx)

consumerId := "0"

// consumer chain has only non-active validators
err := providerKeeper.SetConsumerValSet(ctx, consumerId, []providertypes.ConsensusValidator{
{ProviderConsAddr: consensusAddresses[3]},
{ProviderConsAddr: consensusAddresses[4]}})
require.NoError(t, err)
hasActiveValidatorOptedIn, err := providerKeeper.HasActiveConsumerValidator(ctx, consumerId, activeValidators)
require.NoError(t, err)
require.False(t, hasActiveValidatorOptedIn)

// consumer chain has one active validator
err = providerKeeper.SetConsumerValSet(ctx, consumerId, []providertypes.ConsensusValidator{
{ProviderConsAddr: consensusAddresses[2]}})
require.NoError(t, err)
hasActiveValidatorOptedIn, err = providerKeeper.HasActiveConsumerValidator(ctx, consumerId, activeValidators)
require.NoError(t, err)
require.True(t, hasActiveValidatorOptedIn)

// consumer chain has one active and two non-active validators
err = providerKeeper.SetConsumerValSet(ctx, consumerId, []providertypes.ConsensusValidator{
{ProviderConsAddr: consensusAddresses[3]},
{ProviderConsAddr: consensusAddresses[4]},
{ProviderConsAddr: consensusAddresses[1]}})
require.NoError(t, err)
hasActiveValidatorOptedIn, err = providerKeeper.HasActiveConsumerValidator(ctx, consumerId, activeValidators)
require.NoError(t, err)
require.True(t, hasActiveValidatorOptedIn)
}

func TestCreateConsumerClient(t *testing.T) {
type testCase struct {
description string
Expand Down
Loading