Provider panic cleanup (#647)

* EndBlockCCR and StopConsumerChain

* audit genesis.go panics

* GetMaturedUnbondingOps panics instead of returning error

* completeMaturedUnbondingOps done

* replace ApplyKeyAssignmentToValUpdates with MustApplyKeyAssignmentToValUpdates

* panics in keeper - wip

* add RemoveConsumerFromUnbondingOp

* add TODO for GenesisState validation

* add ValidateUnbondingOp

* hooks panics

* make sure ConsumerGenesis is serializable

* panics in proposal.go

* add TODO for failed VSCPacket sends

* fix linter

* remove redundant code

* apply review suggestions

* add TestHandleVSCMaturedPacket

* fix TestRemoveConsumerFromUnbondingOp

* fix linter

tests/e2e/common.go

@@ -300,13 +300,9 @@ func checkStakingUnbondingOps(s *CCVTestSuite, id uint64, found bool, onHold boo
 }
 
 func checkCCVUnbondingOp(s *CCVTestSuite, providerCtx sdk.Context, chainID string, valUpdateID uint64, found bool, msgAndArgs ...interface{}) {
-	entries, wasFound := s.providerApp.GetProviderKeeper().GetUnbondingOpsFromIndex(providerCtx, chainID, valUpdateID)
-	s.Require().Equal(
-		found,
-		wasFound,
-		fmt.Sprintf("checkCCVUnbondingOp failed - GetUnbondingOpsFromIndex; %s", msgAndArgs...),
-	)
+	entries := s.providerApp.GetProviderKeeper().GetUnbondingOpsFromIndex(providerCtx, chainID, valUpdateID)
 	if found {
+		s.Require().NotEmpty(entries, fmt.Sprintf("checkCCVUnbondingOp failed - should not be empty; %s", msgAndArgs...))
 		s.Require().Greater(
 			len(entries),
 			0,

x/ccv/provider/keeper/genesis.go

@@ -19,7 +19,8 @@ func (k Keeper) InitGenesis(ctx sdk.Context, genState *types.GenesisState) {
 		// and claims the returned capability
 		err := k.BindPort(ctx, ccv.ProviderPortID)
 		if err != nil {
-			panic(fmt.Sprintf("could not claim port capability: %v", err))
+			// If the binding fails, the chain MUST NOT start
+			panic(fmt.Errorf("could not claim port capability: %v", err))
 		}
 	}
 
@@ -52,6 +53,8 @@ func (k Keeper) InitGenesis(ctx sdk.Context, genState *types.GenesisState) {
 		chainID := cs.ChainId
 		k.SetConsumerClientId(ctx, chainID, cs.ClientId)
 		if err := k.SetConsumerGenesis(ctx, chainID, cs.ConsumerGenesis); err != nil {
+			// An error here would indicate something is very wrong,
+			// the ConsumerGenesis validated in ConsumerState.Validate().
 			panic(fmt.Errorf("consumer chain genesis could not be persisted: %w", err))
 		}
 		for _, ubdOpIndex := range cs.UnbondingOpsIndex {
@@ -114,7 +117,7 @@ func (k Keeper) ExportGenesis(ctx sdk.Context) *types.GenesisState {
 			cs.ChannelId = channelId
 			cs.InitialHeight, found = k.GetInitChainHeight(ctx, chain.ChainId)
 			if !found {
-				panic(fmt.Errorf("cannot find genesis for consumer chain %s with client %s", chain.ChainId, chain.ClientId))
+				panic(fmt.Errorf("cannot find init height for consumer chain %s", chain.ChainId))
 			}
 			cs.SlashDowntimeAck = k.GetSlashAcks(ctx, chain.ChainId)
 		}
@@ -124,11 +127,6 @@ func (k Keeper) ExportGenesis(ctx sdk.Context) *types.GenesisState {
 
 	}
 
-	matureUbdOps, err := k.GetMaturedUnbondingOps(ctx)
-	if err != nil {
-		panic(err)
-	}
-
 	// ConsumerAddrsToPrune are added only for registered consumer chains
 	consumerAddrsToPrune := []types.ConsumerAddrsToPrune{}
 	for _, chain := range registeredChains {
@@ -142,7 +140,7 @@ func (k Keeper) ExportGenesis(ctx sdk.Context) *types.GenesisState {
 		k.GetAllValsetUpdateBlockHeights(ctx),
 		consumerStates,
 		k.GetAllUnbondingOps(ctx),
-		&ccv.MaturedUnbondingOps{Ids: matureUbdOps},
+		&ccv.MaturedUnbondingOps{Ids: k.GetMaturedUnbondingOps(ctx)},
 		k.GetAllPendingConsumerAdditionProps(ctx),
 		k.GetAllPendingConsumerRemovalProps(ctx),
 		params,

x/ccv/provider/keeper/genesis_test.go

@@ -130,8 +130,7 @@ func TestInitAndExportGenesis(t *testing.T) {
 	ubdOps, found := pk.GetUnbondingOp(ctx, vscID)
 	require.True(t, found)
 	require.Equal(t, provGenesis.UnbondingOps[0], ubdOps)
-	matureUbdOps, err := pk.GetMaturedUnbondingOps(ctx)
-	require.NoError(t, err)
+	matureUbdOps := pk.GetMaturedUnbondingOps(ctx)
 	require.Equal(t, ubdIndex, matureUbdOps)
 	chainID, found := pk.GetChannelToChain(ctx, provGenesis.ConsumerStates[0].ChannelId)
 	require.True(t, found)

x/ccv/provider/keeper/hooks.go

@@ -53,21 +53,32 @@ func (h Hooks) AfterUnbondingInitiated(ctx sdk.Context, ID uint64) error {
 	if err := h.k.stakingKeeper.PutUnbondingOnHold(ctx, ID); err != nil {
 		// If there was an error putting the unbonding on hold, panic to end execution for
 		// the current tx and prevent committal of this invalid state.
+		//
+		// Note: that in the case of a validator unbonding, AfterUnbondingInitiated is called
+		// form staking.EndBlock, thus the following panic would halt the chain.
+		// In this case PutUnbondingOnHold fails if either the unbonding operation was
+		// not found or the UnbondingOnHoldRefCount is negative. In either cases,
+		// the state of the x/staking module of cosmos-sdk is invalid.
 		panic(fmt.Errorf("unbonding could not be put on hold: %w", err))
 	}
 	return nil
 }
 
+// ValidatorConsensusKeyInUse is called when a new validator is created
+// in the x/staking module of cosmos-sdk. In case it panics, the TX aborts
+// and thus, the validator is not created. See AfterValidatorCreated hook.
 func ValidatorConsensusKeyInUse(k *Keeper, ctx sdk.Context, valAddr sdk.ValAddress) bool {
 	// Get the validator being added in the staking module.
 	val, found := k.stakingKeeper.GetValidator(ctx, valAddr)
 	if !found {
+		// Abort TX, do NOT allow validator to be created
 		panic("did not find newly created validator in staking module")
 	}
 
 	// Get the consensus address of the validator being added
 	consensusAddr, err := val.GetConsAddr()
 	if err != nil {
+		// Abort TX, do NOT allow validator to be created
 		panic("could not get validator cons addr ")
 	}