Feat: Add val denylist (quicksilver-zone#1268)

* initial work

* refactor

* remove new msg type

* [wip]: add query type

* add grpc query

* add crud tests for val deny list

* refactor deny list storage

Now store validator address bytes instead of serialized form of the Validator struct

* filter validator based on denylist

* linting

* add initial supply if user doesn't have balance

* add comment

* Add grpc test for query deny list

* Update x/interchainstaking/keeper/intent_test.go

Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>

* refactor based on review, return string instead of validator object, using sdk.ValAddress as argument

* linting

* refactor: remove unused marshal code

---------

Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>

proto/quicksilver/interchainstaking/v1/query.proto

@@ -255,3 +255,11 @@ message QueryMappedAccountsResponse {
   map<string, bytes> RemoteAddressMap = 1 [(gogoproto.nullable) = false];
   cosmos.base.query.v1beta1.PageResponse pagination = 2;
 }
+
+message QueryDenyListRequest {
+  string chain_id = 1 [(gogoproto.moretags) = "yaml:\"chain_id\""];
+}
+message QueryDenyListResponse {
+  repeated string validators = 1 [(cosmos_proto.scalar) = "cosmos.AddressString"];
+  cosmos.base.query.v1beta1.PageResponse pagination = 2;
+}

x/interchainstaking/keeper/deny_list.go

@@ -0,0 +1,60 @@
+package keeper
+
+import (
+	sdk "github.com/cosmos/cosmos-sdk/types"
+
+	"github.com/quicksilver-zone/quicksilver/utils/addressutils"
+	"github.com/quicksilver-zone/quicksilver/x/interchainstaking/types"
+)
+
+// SetZoneValidatorToDenyList sets the zone validator deny list
+func (k *Keeper) SetZoneValidatorToDenyList(ctx sdk.Context, chainID string, validatorAddress sdk.ValAddress) error {
+	store := ctx.KVStore(k.storeKey)
+
+	key := types.GetDeniedValidatorKey(chainID, validatorAddress)
+	store.Set(key, validatorAddress)
+	return nil
+}
+
+// GetZoneValidatorDenyList get the validator deny list of a specific zone
+func (k *Keeper) GetZoneValidatorDenyList(ctx sdk.Context, chainID string) (denyList []string) {
+	zone, found := k.GetZone(ctx, chainID)
+	if !found {
+		return denyList
+	}
+	k.IterateZoneDeniedValidator(ctx, chainID, func(validator sdk.ValAddress) bool {
+		denyList = append(denyList, addressutils.MustEncodeAddressToBech32(zone.GetValoperPrefix(), validator))
+		return false
+	})
+
+	return denyList
+}
+
+func (k *Keeper) GetDeniedValidatorInDenyList(ctx sdk.Context, chainID string, validatorAddress sdk.ValAddress) bool {
+	key := types.GetDeniedValidatorKey(chainID, validatorAddress)
+	store := ctx.KVStore(k.storeKey)
+	bz := store.Get(key)
+	return bz != nil
+}
+
+// RemoveValidatorFromDenyList removes a validator from the deny list. Panic if the validator is not in the deny list
+func (k *Keeper) RemoveValidatorFromDenyList(ctx sdk.Context, chainID string, validator sdk.ValAddress) {
+	store := ctx.KVStore(k.storeKey)
+	key := types.GetDeniedValidatorKey(chainID, validator)
+	store.Delete(key)
+}
+
+func (k *Keeper) IterateZoneDeniedValidator(ctx sdk.Context, chainID string, cb func(validator sdk.ValAddress) (stop bool)) {
+	store := ctx.KVStore(k.storeKey)
+	deniedValPrefixKey := types.GetZoneDeniedValidatorKey(chainID)
+
+	iterator := sdk.KVStorePrefixIterator(store, deniedValPrefixKey)
+	defer iterator.Close()
+
+	for ; iterator.Valid(); iterator.Next() {
+		validator := sdk.ValAddress(iterator.Value())
+		if cb(validator) {
+			break
+		}
+	}
+}

x/interchainstaking/keeper/deny_list_test.go

@@ -0,0 +1,103 @@
+package keeper_test
+
+import (
+	sdk "github.com/cosmos/cosmos-sdk/types"
+
+	"github.com/quicksilver-zone/quicksilver/utils/addressutils"
+)
+
+func (suite *KeeperTestSuite) TestStoreGetDeleteDenyList() {
+	suite.Run("deny list - store / get / delete single", func() {
+		suite.SetupTest()
+		suite.setupTestZones()
+
+		qApp := suite.GetQuicksilverApp(suite.chainA)
+		ctx := suite.chainA.GetContext()
+
+		zone, found := qApp.InterchainstakingKeeper.GetZone(ctx, suite.chainB.ChainID)
+		suite.True(found)
+		vals := qApp.InterchainstakingKeeper.GetValidators(ctx, zone.ChainId)
+		suite.Len(vals, 4)
+		val := vals[0]
+		validator := sdk.ValAddress(val.ValoperAddress)
+		// Initially the deny list should be empty
+		found = qApp.InterchainstakingKeeper.GetDeniedValidatorInDenyList(ctx, zone.ChainId, validator)
+		suite.False(found)
+		// Add a validator to the deny list
+		err := qApp.InterchainstakingKeeper.SetZoneValidatorToDenyList(ctx, zone.ChainId, validator)
+		suite.NoError(err)
+
+		// Ensure the deny list contains the validator
+
+		denyList := qApp.InterchainstakingKeeper.GetZoneValidatorDenyList(ctx, zone.ChainId)
+		suite.Len(denyList, 1)
+		suite.Equal(addressutils.MustEncodeAddressToBech32(zone.GetValoperPrefix(), validator), denyList[0])
+
+		found = qApp.InterchainstakingKeeper.GetDeniedValidatorInDenyList(ctx, zone.ChainId, validator)
+		suite.True(found)
+
+		// Remove the validator from the deny list
+		qApp.InterchainstakingKeeper.RemoveValidatorFromDenyList(ctx, zone.ChainId, validator)
+		found = qApp.InterchainstakingKeeper.GetDeniedValidatorInDenyList(ctx, zone.ChainId, validator)
+		suite.False(found)
+
+		denyList = qApp.InterchainstakingKeeper.GetZoneValidatorDenyList(ctx, zone.ChainId)
+		suite.Len(denyList, 0)
+	})
+
+	suite.Run("deny list - store / get / delete multiple", func() {
+		suite.SetupTest()
+		suite.setupTestZones()
+
+		qApp := suite.GetQuicksilverApp(suite.chainA)
+		ctx := suite.chainA.GetContext()
+
+		zone, found := qApp.InterchainstakingKeeper.GetZone(ctx, suite.chainB.ChainID)
+		suite.True(found)
+		vals := qApp.InterchainstakingKeeper.GetValidators(ctx, zone.ChainId)
+		suite.Len(vals, 4)
+		valAddr := make([]sdk.ValAddress, 4)
+		for i, v := range vals {
+			valAddr[i] = sdk.ValAddress(v.ValoperAddress)
+		}
+		// Initially the deny list should be empty
+		found = qApp.InterchainstakingKeeper.GetDeniedValidatorInDenyList(ctx, zone.ChainId, valAddr[0])
+		suite.False(found)
+
+		// Add three validators to the deny list
+		err := qApp.InterchainstakingKeeper.SetZoneValidatorToDenyList(ctx, zone.ChainId, valAddr[0])
+		suite.NoError(err)
+		err = qApp.InterchainstakingKeeper.SetZoneValidatorToDenyList(ctx, zone.ChainId, valAddr[1])
+		suite.NoError(err)
+		err = qApp.InterchainstakingKeeper.SetZoneValidatorToDenyList(ctx, zone.ChainId, valAddr[2])
+		suite.NoError(err)
+
+		denyList := qApp.InterchainstakingKeeper.GetZoneValidatorDenyList(ctx, zone.ChainId)
+		suite.Len(denyList, 3)
+
+		// Ensure the deny list contains the three validators
+		suite.Contains(denyList, addressutils.MustEncodeAddressToBech32(zone.GetValoperPrefix(), valAddr[0]))
+		suite.Contains(denyList, addressutils.MustEncodeAddressToBech32(zone.GetValoperPrefix(), valAddr[1]))
+		suite.Contains(denyList, addressutils.MustEncodeAddressToBech32(zone.GetValoperPrefix(), valAddr[2]))
+
+		// Remove the validator from the deny list
+		qApp.InterchainstakingKeeper.RemoveValidatorFromDenyList(ctx, zone.ChainId, valAddr[1])
+		found = qApp.InterchainstakingKeeper.GetDeniedValidatorInDenyList(ctx, zone.ChainId, valAddr[1])
+		suite.False(found)
+
+		// Ensure the deny list contains the two remaining validators
+		denyList = qApp.InterchainstakingKeeper.GetZoneValidatorDenyList(ctx, zone.ChainId)
+		suite.Len(denyList, 2)
+		suite.NotContains(denyList, addressutils.MustEncodeAddressToBech32(zone.GetValoperPrefix(), valAddr[1]))
+		suite.Contains(denyList, addressutils.MustEncodeAddressToBech32(zone.GetValoperPrefix(), valAddr[0]))
+		suite.Contains(denyList, addressutils.MustEncodeAddressToBech32(zone.GetValoperPrefix(), valAddr[2]))
+
+		// Remove the remaining two validators from the deny list
+		qApp.InterchainstakingKeeper.RemoveValidatorFromDenyList(ctx, zone.ChainId, valAddr[0])
+		qApp.InterchainstakingKeeper.RemoveValidatorFromDenyList(ctx, zone.ChainId, valAddr[2])
+
+		// Ensure the deny list is empty
+		denyList = qApp.InterchainstakingKeeper.GetZoneValidatorDenyList(ctx, zone.ChainId)
+		suite.Len(denyList, 0)
+	})
+}

x/interchainstaking/keeper/grpc_query.go

@@ -337,3 +337,14 @@ func (k *Keeper) MappedAccounts(c context.Context, req *types.QueryMappedAccount
 
 	return &types.QueryMappedAccountsResponse{RemoteAddressMap: remoteAddressMap}, nil
 }
+
+func (k *Keeper) ValidatorDenyList(c context.Context, req *types.QueryDenyListRequest) (*types.QueryDenyListResponse, error) {
+	if req == nil {
+		return nil, status.Error(codes.InvalidArgument, "empty request")
+	}
+	ctx := sdk.UnwrapSDKContext(c)
+
+	validators := k.GetZoneValidatorDenyList(ctx, req.ChainId)
+
+	return &types.QueryDenyListResponse{Validators: validators}, nil
+}

x/interchainstaking/keeper/grpc_query_test.go

@@ -1346,3 +1346,68 @@ func (suite *KeeperTestSuite) TestKeeper_Zone() {
 		})
 	}
 }
+
+func (suite *KeeperTestSuite) TestKeeper_ZoneValidatorDenyList() {
+	testCases := []struct {
+		name           string
+		req            *types.QueryDenyListRequest
+		wantErr        bool
+		expectedLength int
+	}{
+		{
+			name:           "empty request",
+			req:            nil,
+			wantErr:        true,
+			expectedLength: 0,
+		},
+		{
+			name:           "zone not found",
+			req:            &types.QueryDenyListRequest{ChainId: "abcd"},
+			wantErr:        false,
+			expectedLength: 0,
+		},
+		{
+			name:           "zone valid request",
+			req:            &types.QueryDenyListRequest{ChainId: suite.chainB.ChainID},
+			wantErr:        false,
+			expectedLength: 2,
+		},
+	}
+	for _, tc := range testCases {
+		suite.Run(tc.name, func() {
+			suite.SetupTest()
+			suite.setupTestZones()
+
+			quicksilver := suite.GetQuicksilverApp(suite.chainA)
+			ctx := suite.chainA.GetContext()
+			icsKeeper := quicksilver.InterchainstakingKeeper
+
+			// Set 2 validators to deny list
+			validator1 := icsKeeper.GetValidators(ctx, suite.chainB.ChainID)[0]
+			valAddr, err := sdk.ValAddressFromBech32(validator1.ValoperAddress)
+			suite.NoError(err)
+			err = icsKeeper.SetZoneValidatorToDenyList(ctx, suite.chainB.ChainID, valAddr)
+			suite.NoError(err)
+
+			validator2 := icsKeeper.GetValidators(ctx, suite.chainB.ChainID)[1]
+			valAddr, err = sdk.ValAddressFromBech32(validator2.ValoperAddress)
+			suite.NoError(err)
+			err = icsKeeper.SetZoneValidatorToDenyList(ctx, suite.chainB.ChainID, valAddr)
+			suite.NoError(err)
+			denyList, err := icsKeeper.ValidatorDenyList(ctx, tc.req)
+			if tc.wantErr {
+				suite.T().Logf("Error:\n%v\n", err)
+				suite.Error(err)
+				suite.Empty(denyList)
+			} else {
+				suite.NotNil(denyList)
+				if tc.expectedLength == 2 {
+					suite.Equal(&types.QueryDenyListResponse{Validators: []string{validator1.ValoperAddress, validator2.ValoperAddress}}, denyList)
+				} else {
+					suite.Empty(denyList.Validators)
+				}
+
+			}
+		})
+	}
+}

x/interchainstaking/keeper/intent_test.go

@@ -324,7 +324,6 @@ func (suite *KeeperTestSuite) TestAggregateIntent() {
 			icsKeeper := quicksilver.InterchainstakingKeeper
 			zone, found := icsKeeper.GetZone(ctx, suite.chainB.ChainID)
 			suite.True(found)
-
 			// give each user some funds
 			for addrString, balance := range tt.balances() {
 				suite.giveFunds(ctx, zone.LocalDenom, balance, addrString)
@@ -334,7 +333,13 @@ func (suite *KeeperTestSuite) TestAggregateIntent() {
 				icsKeeper.SetDelegatorIntent(ctx, &zone, intent, false)
 			}
 
-			_ = icsKeeper.AggregateDelegatorIntents(ctx, &zone)
+			// If the supply is zero, mint some coins to avoid zero ordializedSum
+			if quicksilver.BankKeeper.GetSupply(ctx, zone.LocalDenom).IsZero() {
+				err := quicksilver.MintKeeper.MintCoins(ctx, sdk.NewCoins(sdk.NewCoin(zone.LocalDenom, sdk.NewInt(1000))))
+				suite.NoError(err)
+			}
+			err := icsKeeper.AggregateDelegatorIntents(ctx, &zone)
+			suite.NoError(err)
 
 			// refresh zone to pull new aggregate
 			zone, found = icsKeeper.GetZone(ctx, suite.chainB.ChainID)

x/interchainstaking/keeper/keeper.go

@@ -4,6 +4,7 @@ import (
 	"bytes"
 	"errors"
 	"fmt"
+	"slices"
 	"time"
 
 	"github.com/tendermint/tendermint/libs/log"
@@ -739,7 +740,7 @@ func (k *Keeper) GetAggregateIntentOrDefault(ctx sdk.Context, zone *types.Zone)
 	}
 
 	jailedThreshold := k.EpochsKeeper.GetEpochInfo(ctx, "epoch").Duration * 2
-
+	denyList := k.GetZoneValidatorDenyList(ctx, zone.ChainId)
 	// filter intents here...
 	// check validators for tombstoned
 	for _, validatorIntent := range intents {
@@ -764,9 +765,9 @@ func (k *Keeper) GetAggregateIntentOrDefault(ctx sdk.Context, zone *types.Zone)
 		}
 
 		// we should never let denylist validators into the list, even if they are explicitly selected
-		// if in deny list {
-		// continue
-		// }
+		if slices.Contains(denyList, validator.ValoperAddress) {
+			continue
+		}
 		filteredIntents = append(filteredIntents, validatorIntent)
 	}
 

x/interchainstaking/types/deny_list.go

@@ -0,0 +1 @@
+package types

x/interchainstaking/types/deny_list_test.go

@@ -0,0 +1 @@
+package types_test

x/interchainstaking/types/error.go

@@ -4,4 +4,7 @@ import (
 	"errors"
 )
 
-var ErrCoinAmountNil = errors.New("coin amount is nil")
+var (
+	ErrCoinAmountNil              = errors.New("coin amount is nil")
+	ErrValidatorAlreadyInDenyList = errors.New("validator already in deny list")
+)

x/interchainstaking/types/keys.go

@@ -63,6 +63,7 @@ var (
 	KeyPrefixRedelegationRecord          = []byte{0x10}
 	KeyPrefixLsmCaps                     = []byte{0x11}
 	KeyPrefixLocalDenomZoneMapping       = []byte{0x12}
+	KeyPrefixDeniedValidator             = []byte{0x13}
 )
 
 // ParseStakingDelegationKey parses the KV store key for a delegation from Cosmos x/staking module,
@@ -165,3 +166,13 @@ func GetRemoteAddressPrefix(locaAddress []byte) []byte {
 func GetZoneValidatorAddrsByConsAddrKey(chainID string) []byte {
 	return append(KeyPrefixValidatorAddrsByConsAddr, []byte(chainID)...)
 }
+
+// GetDeniedValidatorKey gets the validator deny list key prefix for a given chain.
+func GetDeniedValidatorKey(chainID string, validatorAddress sdk.ValAddress) []byte {
+	return append(KeyPrefixDeniedValidator, append([]byte(chainID), validatorAddress.Bytes()...)...)
+}
+
+// GetZoneValidatorDenyListKey gets the validator deny list key prefix for a given chain.
+func GetZoneDeniedValidatorKey(chainID string) []byte {
+	return append(KeyPrefixDeniedValidator, []byte(chainID)...)
+}