apply review suggestions

cosmos · Jul 26, 2024 · 80e90f8 · 80e90f8
1 parent 42cb3ad
commit 80e90f8
Show file tree

Hide file tree

Showing 2 changed files with 16 additions and 7 deletions.
diff --git a/x/ccv/provider/keeper/key_assignment.go b/x/ccv/provider/keeper/key_assignment.go
@@ -318,10 +318,14 @@ func (k Keeper) ConsumeConsumerAddrsToPrune(
 		if _, pruneTs, err := types.ParseChainIdAndTsKey(consumerAddrsToPruneKeyPrefix, iterator.Key()); err != nil {
 			// An error here would indicate something is very wrong,
 			// store keys are assumed to be correctly serialized in AppendConsumerAddrsToPrune.
-			panic(err)
+			k.Logger(ctx).Error("ParseChainIdAndTsKey failed",
+				"key", string(iterator.Key()),
+				"error", err.Error(),
+			)
+			continue
 		} else if pruneTs.After(ts) {
 			// An error here would indicate something is wrong the iterator
-			k.Logger(ctx).Error("iterator in GetConsumerAddrsToPrune failed", "key", string(iterator.Key()))
+			k.Logger(ctx).Error("iterator in ConsumeConsumerAddrsToPrune failed", "key", string(iterator.Key()))
 			continue
 		}
 
@@ -331,7 +335,11 @@ func (k Keeper) ConsumeConsumerAddrsToPrune(
 		if err := addrs.Unmarshal(iterator.Value()); err != nil {
 			// An error here would indicate something is very wrong,
 			// the list of consumer addresses is assumed to be correctly serialized in AppendConsumerAddrsToPrune.
-			panic(err)
+			k.Logger(ctx).Error("unmarshaling in ConsumeConsumerAddrsToPrune failed",
+				"key", string(iterator.Key()),
+				"error", err.Error(),
+			)
+			continue
 		}
 
 		consumerAddrsToPrune.Addresses = append(consumerAddrsToPrune.Addresses, addrs.Addresses...)

diff --git a/x/ccv/provider/keeper/key_assignment_test.go b/x/ccv/provider/keeper/key_assignment_test.go
@@ -225,15 +225,16 @@ func TestConsumerAddrsToPruneCRUD(t *testing.T) {
 
 	keeper.AppendConsumerAddrsToPrune(ctx, chainID, ts1, consumerAddr1)
 
-	addrsToPrune = keeper.ConsumeConsumerAddrsToPrune(ctx, chainID, ts2).Addresses
+	addrsToPrune = keeper.ConsumeConsumerAddrsToPrune(ctx, chainID, ts1).Addresses
 	require.NotEmpty(t, addrsToPrune, "addresses to prune was returned")
-	require.Len(t, addrsToPrune, 2, "addresses to prune is not len 2")
+	require.Len(t, addrsToPrune, 1, "addresses to prune is not len 1")
 	require.Equal(t, addrsToPrune[0], consumerAddr1.ToSdkConsAddr().Bytes())
-	require.Equal(t, addrsToPrune[1], consumerAddr2.ToSdkConsAddr().Bytes())
 	addrsToPrune = keeper.GetConsumerAddrsToPrune(ctx, chainID, ts1).Addresses
 	require.Empty(t, addrsToPrune, "addresses to prune was returned")
 	addrsToPrune = keeper.GetConsumerAddrsToPrune(ctx, chainID, ts2).Addresses
-	require.Empty(t, addrsToPrune, "addresses to prune was returned")
+	require.NotEmpty(t, addrsToPrune, "addresses to prune is empty")
+	require.Len(t, addrsToPrune, 1, "addresses to prune is not len 1")
+	require.Equal(t, addrsToPrune[0], consumerAddr2.ToSdkConsAddr().Bytes())
 }
 
 func TestGetAllConsumerAddrsToPrune(t *testing.T) {