Cleanup related to #296 (#304)

* Cleanup related to #296 - rewrote comment - added "found" booleans to getter methods to clarify logic * fix tests Co-authored-by: Marius Poke <[email protected]>
cosmos · Aug 31, 2022 · fe7353d · fe7353d
1 parent 396869a
commit fe7353d
Show file tree

Hide file tree

Showing 4 changed files with 22 additions and 17 deletions.
diff --git a/e2e-tests/slashing_test.go b/e2e-tests/slashing_test.go
@@ -328,8 +328,8 @@ func (suite *ProviderKeeperTestSuite) TestHandleSlashPacketErrors() {
 	// save VSC ID
 	vID := ProviderKeeper.GetValidatorSetUpdateId(suite.ctx)
 
-	// set faulty block height for current VSC ID
-	ProviderKeeper.SetValsetUpdateBlockHeight(suite.ctx, vID, 0)
+	// remove block height for current VSC ID
+	ProviderKeeper.DeleteValsetUpdateBlockHeight(suite.ctx, vID)
 
 	// expect an error if block height mapping VSC ID is zero
 	_, err = ProviderKeeper.HandleSlashPacket(suite.ctx, consumerChainID, ccv.SlashPacketData{ValsetUpdateId: vID})

diff --git a/x/ccv/provider/keeper/keeper.go b/x/ccv/provider/keeper/keeper.go
@@ -565,13 +565,13 @@ func (k Keeper) SetValsetUpdateBlockHeight(ctx sdk.Context, valsetUpdateId, bloc
 }
 
 // GetValsetUpdateBlockHeight gets the block height for a given valset update id
-func (k Keeper) GetValsetUpdateBlockHeight(ctx sdk.Context, valsetUpdateId uint64) uint64 {
+func (k Keeper) GetValsetUpdateBlockHeight(ctx sdk.Context, valsetUpdateId uint64) (uint64, bool) {
 	store := ctx.KVStore(k.storeKey)
 	bz := store.Get(types.ValsetUpdateBlockHeightKey(valsetUpdateId))
 	if bz == nil {
-		return 0
+		return 0, false
 	}
-	return binary.BigEndian.Uint64(bz)
+	return binary.BigEndian.Uint64(bz), true
 }
 
 // DeleteValsetUpdateBlockHeight deletes the block height value for a given vaset update id
@@ -661,14 +661,14 @@ func (k Keeper) SetInitChainHeight(ctx sdk.Context, chainID string, height uint6
 }
 
 // GetInitChainHeight returns the provider block height when the given consumer chain was initiated
-func (k Keeper) GetInitChainHeight(ctx sdk.Context, chainID string) uint64 {
+func (k Keeper) GetInitChainHeight(ctx sdk.Context, chainID string) (uint64, bool) {
 	store := ctx.KVStore(k.storeKey)
 	bz := store.Get(types.InitChainHeightKey(chainID))
 	if bz == nil {
-		return 0
+		return 0, false
 	}
 
-	return binary.BigEndian.Uint64(bz)
+	return binary.BigEndian.Uint64(bz), true
 }
 
 // DeleteInitChainHeight deletes the block height value for which the given consumer chain's channel was established

diff --git a/x/ccv/provider/keeper/keeper_test.go b/x/ccv/provider/keeper/keeper_test.go
@@ -26,20 +26,24 @@ import (
 func TestValsetUpdateBlockHeight(t *testing.T) {
 	providerKeeper, ctx := testkeeper.GetProviderKeeperAndCtx(t)
 
-	blockHeight := providerKeeper.GetValsetUpdateBlockHeight(ctx, uint64(0))
+	blockHeight, found := providerKeeper.GetValsetUpdateBlockHeight(ctx, uint64(0))
+	require.False(t, found)
 	require.Zero(t, blockHeight)
 
 	providerKeeper.SetValsetUpdateBlockHeight(ctx, uint64(1), uint64(2))
-	blockHeight = providerKeeper.GetValsetUpdateBlockHeight(ctx, uint64(1))
+	blockHeight, found = providerKeeper.GetValsetUpdateBlockHeight(ctx, uint64(1))
+	require.True(t, found)
 	require.Equal(t, blockHeight, uint64(2))
 
 	providerKeeper.DeleteValsetUpdateBlockHeight(ctx, uint64(1))
-	blockHeight = providerKeeper.GetValsetUpdateBlockHeight(ctx, uint64(1))
+	blockHeight, found = providerKeeper.GetValsetUpdateBlockHeight(ctx, uint64(1))
+	require.False(t, found)
 	require.Zero(t, blockHeight)
 
 	providerKeeper.SetValsetUpdateBlockHeight(ctx, uint64(1), uint64(2))
 	providerKeeper.SetValsetUpdateBlockHeight(ctx, uint64(3), uint64(4))
-	blockHeight = providerKeeper.GetValsetUpdateBlockHeight(ctx, uint64(3))
+	blockHeight, found = providerKeeper.GetValsetUpdateBlockHeight(ctx, uint64(3))
+	require.True(t, found)
 	require.Equal(t, blockHeight, uint64(4))
 }
 
@@ -175,7 +179,7 @@ func TestInitHeight(t *testing.T) {
 	providerKeeper.SetInitChainHeight(ctx, tc[2].chainID, tc[2].expected)
 
 	for _, tc := range tc {
-		height := providerKeeper.GetInitChainHeight(ctx, tc.chainID)
+		height, _ := providerKeeper.GetInitChainHeight(ctx, tc.chainID)
 		require.Equal(t, tc.expected, height)
 	}
 }

diff --git a/x/ccv/provider/keeper/relay.go b/x/ccv/provider/keeper/relay.go
@@ -205,14 +205,15 @@ func (k Keeper) OnRecvSlashPacket(ctx sdk.Context, packet channeltypes.Packet, d
 func (k Keeper) HandleSlashPacket(ctx sdk.Context, chainID string, data ccv.SlashPacketData) (success bool, err error) {
 	// map VSC ID to infraction height for the given chain ID
 	var infractionHeight uint64
+	var found bool
 	if data.ValsetUpdateId == 0 {
-		infractionHeight = k.GetInitChainHeight(ctx, chainID)
+		infractionHeight, found = k.GetInitChainHeight(ctx, chainID)
 	} else {
-		infractionHeight = k.GetValsetUpdateBlockHeight(ctx, data.ValsetUpdateId)
+		infractionHeight, found = k.GetValsetUpdateBlockHeight(ctx, data.ValsetUpdateId)
 	}
 
-	// return if there isn't any initial chain height for the consumer chain
-	if infractionHeight == 0 {
+	// return error if we cannot find infraction height matching the validator update id
+	if !found {
 		return false, fmt.Errorf("cannot find infraction height matching the validator update id %d for chain %s", data.ValsetUpdateId, chainID)
 	}