Standalone to consumer changeover - staking functionalities

app/consumer-democracy/app.go

@@ -427,7 +427,7 @@ func New(
 		authtypes.FeeCollectorName,
 	)
 
-	// Setting the staking keeper is only needed for standalone to consumer changeover chains
+	// Setting the standalone staking keeper is only needed for standalone to consumer changeover chains
 	app.ConsumerKeeper.SetStandaloneStakingKeeper(app.StakingKeeper)
 
 	// consumer keeper satisfies the staking keeper interface

tests/integration/slashing.go

@@ -665,3 +665,39 @@ func (suite *CCVTestSuite) TestQueueAndSendSlashPacket() {
 	suite.Require().Empty(dataPackets)
 	suite.Require().Len(dataPackets.GetList(), 0)
 }
+
+// TestCISBeforeCCVEstablished tests that the consumer chain doesn't panic or
+// have any undesired behavior when a slash packet is queued before the CCV channel is established.
+// Then once the CCV channel is established, the slash packet should be sent soon after.
+func (suite *CCVTestSuite) TestCISBeforeCCVEstablished() {
+	consumerKeeper := suite.consumerApp.GetConsumerKeeper()
+
+	// Check pending packets is empty
+	pendingPackets := consumerKeeper.GetPendingPackets(suite.consumerCtx())
+	suite.Require().Len(pendingPackets.List, 0)
+
+	consumerKeeper.Slash(suite.consumerCtx(), []byte{0x01, 0x02, 0x3},
+		66, 4324, sdk.MustNewDecFromStr("0.05"), stakingtypes.Downtime)
+
+	// Check slash packet was queued
+	pendingPackets = consumerKeeper.GetPendingPackets(suite.consumerCtx())
+	suite.Require().Len(pendingPackets.List, 1)
+
+	// Pass 5 blocks, confirming the consumer doesn't panic
+	for i := 0; i < 5; i++ {
+		suite.consumerChain.NextBlock()
+	}
+
+	// Check packet is still queued
+	pendingPackets = consumerKeeper.GetPendingPackets(suite.consumerCtx())
+	suite.Require().Len(pendingPackets.List, 1)
+
+	// establish ccv channel
+	suite.SetupCCVChannel(suite.path)
+	suite.SendEmptyVSCPacket()
+
+	// Pass one more block, and confirm the packet is sent now that ccv channel is established
+	suite.consumerChain.NextBlock()
+	pendingPackets = consumerKeeper.GetPendingPackets(suite.consumerCtx())
+	suite.Require().Len(pendingPackets.List, 0)
+}

testutil/integration/debug_test.go

@@ -141,6 +141,10 @@ func TestQueueAndSendSlashPacket(t *testing.T) {
 	runCCVTestByName(t, "TestQueueAndSendSlashPacket")
 }
 
+func TestCISBeforeCCVEstablished(t *testing.T) {
+	runCCVTestByName(t, "TestCISBeforeCCVEstablished")
+}
+
 //
 // Stop consumer tests
 //

x/ccv/consumer/keeper/changeover.go

@@ -5,17 +5,30 @@ import (
 	abci "github.com/tendermint/tendermint/abci/types"
 )
 
+// ChangeoverIsComplete returns whether the standalone to consumer changeover process is complete.
+func (k Keeper) ChangeoverIsComplete(ctx sdk.Context) bool {
+	if !k.IsPrevStandaloneChain() {
+		panic("ChangeoverIsComplete should only be called on previously standalone consumers")
+	}
+	return ctx.BlockHeight() >= k.FirstConsumerHeight(ctx)
+}
+
+// The first height that the ccv valset will be in effect is 2 blocks after init genesis height
+// (aka height that the ccv module first returned updates to tendermint), because if init genesis is block N,
+// the new valset is committed in block N+ValidatorUpdateDelay, and in effect for block N+ValidatorUpdateDelay+1.
+func (k Keeper) FirstConsumerHeight(ctx sdk.Context) int64 {
+	return k.GetInitGenesisHeight(ctx) + sdk.ValidatorUpdateDelay + 1
+}
+
 // ChangeoverToConsumer includes the logic that needs to execute during the process of a
 // standalone to consumer changeover, where the previously standalone chain has
 // just been upgraded to include the consumer ccv module, but the provider valset is not
 // yet responsible for POS/block production. This method constructs validator updates
 // that will be given to tendermint, which allows the consumer chain to
 // start using the provider valset, while the standalone valset is given zero voting power where appropriate.
 func (k Keeper) ChangeoverToConsumer(ctx sdk.Context) (initialValUpdates []abci.ValidatorUpdate) {
-	initialValUpdates = k.GetInitialValSet(ctx)
-	// set last standalone height
-	k.SetLastStandaloneHeight(ctx, ctx.BlockHeight())
 	// populate cross chain validators states with initial valset
+	initialValUpdates = k.GetInitialValSet(ctx)
 	k.ApplyCCValidatorChanges(ctx, initialValUpdates)
 
 	// Add validator updates to initialValUpdates, such that the "old" validators returned from standalone staking module

x/ccv/consumer/keeper/changeover_test.go

@@ -110,9 +110,6 @@ func TestChangeoverToConsumer(t *testing.T) {
 		// PreCCV should now be toggled false
 		require.False(t, consumerKeeper.IsPreCCV(ctx))
 
-		// Last standalone height should be set to current block height
-		require.Equal(t, ctx.BlockHeight(), consumerKeeper.GetLastStandaloneHeight(ctx))
-
 		// Cross chain validator states should be populated with initial valset
 		ccVals := consumerKeeper.GetAllCCValidator(ctx)
 		require.Len(t, ccVals, len(tc.initialValUpdates))

x/ccv/consumer/keeper/genesis.go

@@ -27,6 +27,7 @@ func (k Keeper) InitGenesis(ctx sdk.Context, state *consumertypes.GenesisState)
 		k.SetPreCCVTrue(ctx)
 		k.SetInitialValSet(ctx, state.InitialValSet)
 	}
+	k.SetInitGenesisHeight(ctx, ctx.BlockHeight()) // Usually 0, but not the case for changeover chains
 
 	k.SetParams(ctx, state.Params)
 	// TODO: Remove enabled flag and find a better way to setup integration tests

x/ccv/consumer/keeper/keeper.go

@@ -87,10 +87,16 @@ func NewKeeper(
 	return k
 }
 
+// SetStandaloneStakingKeeper sets the standalone staking keeper for the consumer chain.
+// This method should only be called for previously standalone chains that are now consumers.
 func (k *Keeper) SetStandaloneStakingKeeper(sk ccv.StakingKeeper) {
 	k.standaloneStakingKeeper = sk
 }
 
+func (k Keeper) IsPrevStandaloneChain() bool {
+	return k.standaloneStakingKeeper != nil
+}
+
 // Validates that the consumer keeper is initialized with non-zero and
 // non-nil values for all its fields. Otherwise this method will panic.
 func (k Keeper) mustValidateFields() {
@@ -254,17 +260,17 @@ func (k Keeper) DeletePendingChanges(ctx sdk.Context) {
 	store.Delete(types.PendingChangesKey())
 }
 
-func (k Keeper) GetLastStandaloneHeight(ctx sdk.Context) int64 {
+func (k Keeper) GetInitGenesisHeight(ctx sdk.Context) int64 {
 	store := ctx.KVStore(k.storeKey)
 	bz := store.Get(types.LastStandaloneHeightKey())
 	if bz == nil {
-		return 0
+		panic("last standalone height not set")
 	}
 	height := sdk.BigEndianToUint64(bz)
 	return int64(height)
 }
 
-func (k Keeper) SetLastStandaloneHeight(ctx sdk.Context, height int64) {
+func (k Keeper) SetInitGenesisHeight(ctx sdk.Context, height int64) {
 	bz := sdk.Uint64ToBigEndian(uint64(height))
 	store := ctx.KVStore(k.storeKey)
 	store.Set(types.LastStandaloneHeightKey(), bz)

x/ccv/consumer/keeper/keeper_test.go

@@ -82,21 +82,21 @@ func TestPendingChanges(t *testing.T) {
 	require.Nil(t, gotPd, "got non-nil pending changes after Delete")
 }
 
-// TestLastSovereignHeight tests the getter and setter for the last standalone height
-func TestLastSovereignHeight(t *testing.T) {
+// TestLastSovereignHeight tests the getter and setter for the ccv init genesis height
+func TestInitGenesisHeight(t *testing.T) {
 	consumerKeeper, ctx, ctrl, _ := testkeeper.GetConsumerKeeperAndCtx(t, testkeeper.NewInMemKeeperParams(t))
 	defer ctrl.Finish()
 
-	// Default value should be 0 without any setter
-	require.Equal(t, int64(0), consumerKeeper.GetLastStandaloneHeight(ctx))
+	// Panics without setter
+	require.Panics(t, func() { consumerKeeper.GetInitGenesisHeight(ctx) })
 
-	// Set/get the last standalone height being 10
-	consumerKeeper.SetLastStandaloneHeight(ctx, 10)
-	require.Equal(t, int64(10), consumerKeeper.GetLastStandaloneHeight(ctx))
+	// Set/get the height being 10
+	consumerKeeper.SetInitGenesisHeight(ctx, 10)
+	require.Equal(t, int64(10), consumerKeeper.GetInitGenesisHeight(ctx))
 
-	// Set/get the last standalone height being 43234426
-	consumerKeeper.SetLastStandaloneHeight(ctx, 43234426)
-	require.Equal(t, int64(43234426), consumerKeeper.GetLastStandaloneHeight(ctx))
+	// Set/get the height being 43234426
+	consumerKeeper.SetInitGenesisHeight(ctx, 43234426)
+	require.Equal(t, int64(43234426), consumerKeeper.GetInitGenesisHeight(ctx))
 }
 
 // TestPreCCV tests the getter, setter and deletion methods for the pre-CCV state flag

x/ccv/consumer/keeper/validators.go

@@ -11,10 +11,6 @@ import (
 	abci "github.com/tendermint/tendermint/abci/types"
 )
 
-//
-// TODO: Address https://github.com/cosmos/interchain-security/issues/781 in this file.
-// Particularly, we need to better define which keepers are responsible for slashing capabilities
-// during/after a standalone to consumer changeover.
 //
 // TODO: make unit tests for all of: MVP consumer, democ consumer, and pre-ccv consumer
 // for previously unimplemented methods, if they're implemented to solve the above issue.
@@ -82,6 +78,12 @@ func (k Keeper) Validator(ctx sdk.Context, addr sdk.ValAddress) stakingtypes.Val
 
 // IsJailed returns the outstanding slashing flag for the given validator adddress
 func (k Keeper) IsValidatorJailed(ctx sdk.Context, addr sdk.ConsAddress) bool {
+	// if the changeover is not complete for prev standalone chain,
+	// return the standalone staking keeper's jailed status
+	if k.IsPrevStandaloneChain() && !k.ChangeoverIsComplete(ctx) {
+		return k.standaloneStakingKeeper.IsValidatorJailed(ctx, addr)
+	}
+	// Otherwise, return the ccv consumer keeper's notion of a validator being jailed
 	return k.OutstandingDowntime(ctx, addr)
 }
 
@@ -102,11 +104,20 @@ func (k Keeper) ValidatorByConsAddr(sdk.Context, sdk.ConsAddress) stakingtypes.V
 
 // Slash queues a slashing request for the the provider chain
 // All queued slashing requests will be cleared in EndBlock
-func (k Keeper) Slash(ctx sdk.Context, addr sdk.ConsAddress, infractionHeight, power int64, _ sdk.Dec, infraction stakingtypes.InfractionType) {
+func (k Keeper) Slash(ctx sdk.Context, addr sdk.ConsAddress, infractionHeight, power int64, slashFactor sdk.Dec, infraction stakingtypes.InfractionType) {
 	if infraction == stakingtypes.InfractionEmpty {
 		return
 	}
 
+	// If this is a previously standalone chain and infraction happened before the changeover was completed,
+	// slash only on the standalone staking keeper.
+	if k.IsPrevStandaloneChain() && infractionHeight < k.FirstConsumerHeight(ctx) {
+		k.standaloneStakingKeeper.Slash(ctx, addr, infractionHeight, power, slashFactor, infraction)
+		return
+	}
+
+	// Otherwise infraction happened after the changeover was completed.
+
 	// if this is a downtime infraction and the validator is allowed to
 	// soft opt out, do not queue a slash packet
 	if infraction == stakingtypes.Downtime {
@@ -139,9 +150,17 @@ func (k Keeper) Slash(ctx sdk.Context, addr sdk.ConsAddress, infractionHeight, p
 }
 
 // Jail - unimplemented on CCV keeper
+//
+// This method should be a no-op even during a standalone to consumer changeover.
+// Once the upgrade has happened as a part of the changeover,
+// the provider validator set will soon be in effect, and jailing is n/a.
 func (k Keeper) Jail(ctx sdk.Context, addr sdk.ConsAddress) {}
 
 // Unjail - unimplemented on CCV keeper
+//
+// This method should be a no-op even during a standalone to consumer changeover.
+// Once the upgrade has happened as a part of the changeover,
+// the provider validator set will soon be in effect, and jailing is n/a.
 func (k Keeper) Unjail(sdk.Context, sdk.ConsAddress) {}
 
 // Delegation - unimplemented on CCV keeper

x/ccv/consumer/keeper/validators_test.go

@@ -104,6 +104,87 @@ func TestApplyCCValidatorChanges(t *testing.T) {
 	}
 }
 
+// TestIsValidatorJailed tests the IsValidatorJailed method for a consumer keeper
+func TestIsValidatorJailed(t *testing.T) {
+	consumerKeeper, ctx, ctrl, mocks := testkeeper.GetConsumerKeeperAndCtx(t, testkeeper.NewInMemKeeperParams(t))
+	defer ctrl.Finish()
+
+	// Consumer keeper from test setup should return false for IsPrevStandaloneChain()
+	require.False(t, consumerKeeper.IsPrevStandaloneChain())
+
+	// IsValidatorJailed should return false for an arbitrary consensus address
+	consAddr := []byte{0x01, 0x02, 0x03}
+	require.False(t, consumerKeeper.IsValidatorJailed(ctx, consAddr))
+
+	// Set outstanding downtime for that addr
+	consumerKeeper.SetOutstandingDowntime(ctx, consAddr)
+
+	// Now confirm IsValidatorJailed returns true
+	require.True(t, consumerKeeper.IsValidatorJailed(ctx, consAddr))
+
+	// Next, we set a value for the standalone staking keeper so IsPrevStandaloneChain() returns true
+	consumerKeeper.SetStandaloneStakingKeeper(mocks.MockStakingKeeper)
+	require.True(t, consumerKeeper.IsPrevStandaloneChain())
+
+	// Set init genesis height to current block height so that ChangeoverIsComplete() is false
+	consumerKeeper.SetInitGenesisHeight(ctx, ctx.BlockHeight())
+	require.False(t, consumerKeeper.ChangeoverIsComplete(ctx))
+
+	// At this point, the state of the consumer keeper is s.t. IsValidatorJailed() queries the standalone staking keeper
+
+	// Now mock that a validator is jailed from the standalone staking keeper
+	mocks.MockStakingKeeper.EXPECT().IsValidatorJailed(ctx, consAddr).Return(true).Times(1)
+
+	// Confirm IsValidatorJailed returns true
+	require.True(t, consumerKeeper.IsValidatorJailed(ctx, consAddr))
+}
+
+func TestSlash(t *testing.T) {
+	consumerKeeper, ctx, ctrl, mocks := testkeeper.GetConsumerKeeperAndCtx(t, testkeeper.NewInMemKeeperParams(t))
+	defer ctrl.Finish()
+
+	// If we call slash with infraction type empty, no slash packet will be queued
+	consumerKeeper.Slash(ctx, []byte{0x01, 0x02, 0x03}, 5, 6, sdk.NewDec(9.0), stakingtypes.InfractionEmpty)
+	pendingPackets := consumerKeeper.GetPendingPackets(ctx)
+	require.Len(t, pendingPackets.List, 0)
+
+	// Consumer keeper from test setup should return false for IsPrevStandaloneChain()
+	require.False(t, consumerKeeper.IsPrevStandaloneChain())
+
+	// Now setup a value for vscID mapped to infraction height
+	consumerKeeper.SetHeightValsetUpdateID(ctx, 5, 6)
+
+	// Call slash with valid infraction type and confirm 1 slash packet is queued
+	consumerKeeper.Slash(ctx, []byte{0x01, 0x02, 0x03}, 5, 6, sdk.NewDec(9.0), stakingtypes.Downtime)
+	pendingPackets = consumerKeeper.GetPendingPackets(ctx)
+	require.Len(t, pendingPackets.List, 1)
+
+	// Next, we set a value for the standalone staking keeper so IsPrevStandaloneChain() returns true
+	consumerKeeper.SetStandaloneStakingKeeper(mocks.MockStakingKeeper)
+	require.True(t, consumerKeeper.IsPrevStandaloneChain())
+
+	// At this point, the state of the consumer keeper is s.t.
+	// Slash() calls the standalone staking keeper's Slash()
+
+	// If we call slash with infraction type empty, standalone staking keeper's slash will not be called
+	// (if it was called, test would panic without mocking the call)
+	consumerKeeper.Slash(ctx, []byte{0x01, 0x02, 0x03}, 5, 6, sdk.NewDec(9.0), stakingtypes.InfractionEmpty)
+
+	// Now setup a mock for Slash, and confirm that it is called against
+	// standalone staking keeper with valid infraction type
+	infractionHeight := int64(5)
+	mocks.MockStakingKeeper.EXPECT().Slash(
+		ctx, []byte{0x01, 0x02, 0x03}, infractionHeight, int64(6),
+		sdk.MustNewDecFromStr("0.05"), stakingtypes.Downtime).Times(1)
+
+	// Also setup init genesis height s.t. infraction height is before first consumer height
+	consumerKeeper.SetInitGenesisHeight(ctx, 4)
+	require.Equal(t, consumerKeeper.FirstConsumerHeight(ctx), int64(6))
+
+	consumerKeeper.Slash(ctx, []byte{0x01, 0x02, 0x03}, infractionHeight, 6,
+		sdk.MustNewDecFromStr("0.05"), stakingtypes.Downtime)
+}
+
 // Tests the getter and setter behavior for historical info
 func TestHistoricalInfo(t *testing.T) {
 	keeperParams := testkeeper.NewInMemKeeperParams(t)