Merge pull request cosmos#37 from 0xPolygon/pos-2755

Implement anti-spam mechanism for proposals in gov

Author: avalkov

x/gov/abci.go

@@ -46,8 +46,8 @@ func EndBlocker(ctx sdk.Context, keeper *keeper.Keeper) error {
 			return false, err
 		}
 
-		// TODO HV2: https://polygon.atlassian.net/browse/POS-2755
-		err = keeper.RefundAndDeleteDeposits(ctx, proposal.Id) // refund deposit if proposal got removed without getting 100% of the proposal
+		// Distribute the deposits if the proposal got removed without getting 100% of the proposal
+		err = keeper.DistributeAndDeleteDeposits(ctx, proposal.Id)
 		if err != nil {
 			return false, err
 		}
@@ -135,10 +135,17 @@ func EndBlocker(ctx sdk.Context, keeper *keeper.Keeper) error {
 			return false, err
 		}
 
-		// HV2: heimdall refunds and deletes deposits in all cases of proposal failures, without caring about burnDeposits
-		err = keeper.RefundAndDeleteDeposits(ctx, proposal.Id)
-		if err != nil {
-			return false, err
+		// HV2: heimdall distributes and deletes deposits in all cases of proposal failures, without caring about burnDeposits
+		if passes {
+			err = keeper.RefundAndDeleteDeposits(ctx, proposal.Id)
+			if err != nil {
+				return false, err
+			}
+		} else {
+			err = keeper.DistributeAndDeleteDeposits(ctx, proposal.Id)
+			if err != nil {
+				return false, err
+			}
 		}
 
 		// If an expedited proposal fails, we do not want to update
@@ -315,7 +322,7 @@ func failUnsupportedProposal(
 		return err
 	}
 
-	if err := keeper.RefundAndDeleteDeposits(ctx, proposal.Id); err != nil {
+	if err := keeper.DistributeAndDeleteDeposits(ctx, proposal.Id); err != nil {
 		return err
 	}
 

x/gov/keeper/common_test.go

@@ -5,16 +5,15 @@ import (
 	"testing"
 
 	borTypes "github.com/0xPolygon/heimdall-v2/x/bor/types"
+	chainmanagertypes "github.com/0xPolygon/heimdall-v2/x/chainmanager/types"
 	checkpointTypes "github.com/0xPolygon/heimdall-v2/x/checkpoint/types"
 	milestoneTypes "github.com/0xPolygon/heimdall-v2/x/milestone/types"
+	stakingtypes "github.com/0xPolygon/heimdall-v2/x/stake/types"
 	topupTypes "github.com/0xPolygon/heimdall-v2/x/topup/types"
 
-	chainmanagertypes "github.com/0xPolygon/heimdall-v2/x/chainmanager/types"
-	consensustypes "github.com/cosmos/cosmos-sdk/x/consensus/types"
-	stakingtypes "github.com/cosmos/cosmos-sdk/x/staking/types"
-
 	cmtproto "github.com/cometbft/cometbft/proto/tendermint/types"
 	cmttime "github.com/cometbft/cometbft/types/time"
+
 	"github.com/golang/mock/gomock"
 	"github.com/stretchr/testify/require"
 
@@ -30,6 +29,7 @@ import (
 	moduletestutil "github.com/cosmos/cosmos-sdk/types/module/testutil"
 	authtypes "github.com/cosmos/cosmos-sdk/x/auth/types"
 	banktypes "github.com/cosmos/cosmos-sdk/x/bank/types"
+	consensustypes "github.com/cosmos/cosmos-sdk/x/consensus/types"
 	disttypes "github.com/cosmos/cosmos-sdk/x/distribution/types"
 	"github.com/cosmos/cosmos-sdk/x/gov/keeper"
 	govtestutil "github.com/cosmos/cosmos-sdk/x/gov/testutil"
@@ -111,7 +111,6 @@ func setupGovKeeper(t *testing.T) (
 	}).AnyTimes()
 
 	stakingKeeper.EXPECT().BondDenom(ctx).Return("pol", nil).AnyTimes()
-	stakingKeeper.EXPECT().IterateCurrentValidatorsAndApplyFn(gomock.Any(), gomock.Any()).AnyTimes()
 	stakingKeeper.EXPECT().TokensFromConsensusPower(gomock.Any(), gomock.Any()).AnyTimes()
 	stakingKeeper.EXPECT().ValidatorAddressCodec().Return(address.NewHexCodec()).AnyTimes()
 	distributionKeeper.EXPECT().FundCommunityPool(gomock.Any(), gomock.Any(), gomock.Any()).Return(nil).AnyTimes()

x/gov/keeper/deposit.go

@@ -3,6 +3,7 @@ package keeper
 import (
 	"context"
 	"fmt"
+	"math/rand"
 	"strings"
 
 	"cosmossdk.io/collections"
@@ -14,6 +15,8 @@ import (
 	disttypes "github.com/cosmos/cosmos-sdk/x/distribution/types"
 	"github.com/cosmos/cosmos-sdk/x/gov/types"
 	v1 "github.com/cosmos/cosmos-sdk/x/gov/types/v1"
+
+	stakeTypes "github.com/0xPolygon/heimdall-v2/x/stake/types"
 )
 
 // SetDeposit sets a Deposit to the gov store
@@ -285,6 +288,94 @@ func (keeper Keeper) RefundAndDeleteDeposits(ctx context.Context, proposalID uin
 	})
 }
 
+// DistributeAndDeleteDeposits distributes the deposits evenly among all the active validators and deletes the deposits on a specific proposal.
+func (keeper Keeper) DistributeAndDeleteDeposits(ctx context.Context, proposalID uint64) error {
+	var validatorAddresses []sdk.AccAddress
+
+	err := keeper.sk.IterateCurrentValidatorsAndApplyFn(ctx, func(validator stakeTypes.Validator) bool {
+		validatorAddr, err := sdk.AccAddressFromHex(validator.Signer)
+		if err != nil {
+			keeper.Logger(ctx).Error("Failed to parse validator address from hex", "error", err)
+			return true
+		}
+		validatorAddresses = append(validatorAddresses, validatorAddr)
+		return false
+	})
+	if err != nil {
+		keeper.Logger(ctx).Error("Error iterating over validators", "error", err)
+		return err
+	}
+
+	numValidators := len(validatorAddresses)
+
+	deposits, err := keeper.GetDeposits(ctx, proposalID)
+	if err != nil {
+		keeper.Logger(ctx).Error("Failed to retrieve deposits for proposal", "proposalID", proposalID, "error", err)
+		return err
+	}
+
+	var totalDeposits sdk.Coins
+	var amountPerValidator sdk.Coins
+
+	for _, deposit := range deposits {
+		depositorAddress, err := keeper.authKeeper.AddressCodec().StringToBytes(deposit.Depositor)
+		if err != nil {
+			keeper.Logger(ctx).Error("Failed to decode depositor address", "error", err)
+			return err
+		}
+
+		totalDeposits = totalDeposits.Add(deposit.Amount...)
+
+		for _, coin := range deposit.Amount {
+			amountPerValidatorPerCoin := sdkmath.LegacyNewDecFromInt(coin.Amount).QuoInt64(int64(numValidators)).TruncateInt()
+			amountPerValidator = amountPerValidator.Add(
+				sdk.NewCoin(
+					coin.Denom,
+					amountPerValidatorPerCoin,
+				),
+			)
+		}
+
+		err = keeper.Deposits.Remove(ctx, collections.Join(deposit.ProposalId, sdk.AccAddress(depositorAddress)))
+		if err != nil {
+			keeper.Logger(ctx).Error("Failed to remove deposit", "proposalID", proposalID, "depositor", depositorAddress, "error", err)
+			return err
+		}
+	}
+
+	for _, validator := range validatorAddresses {
+		err = keeper.bankKeeper.SendCoinsFromModuleToAccount(ctx, types.ModuleName, validator, amountPerValidator)
+		if err != nil {
+			keeper.Logger(ctx).Error("Failed to send coins to validator", "error", err)
+			return err
+		}
+	}
+
+	// Calculate any remaining amount due to truncating issues
+	usedAmount := amountPerValidator.MulInt(sdkmath.NewInt(int64(numValidators)))
+	remainingAmount, isAnyNegative := totalDeposits.SafeSub(usedAmount...)
+	if isAnyNegative {
+		keeper.Logger(ctx).Error("subtraction resulted in a negative amount", "totalDeposits", totalDeposits, "usedAmount", usedAmount)
+		return fmt.Errorf("subtraction resulted in a negative amount")
+	}
+
+	if !remainingAmount.IsZero() {
+		// Send remaining amount to a random validator
+		rand := rand.New(rand.NewSource(int64(numValidators)))
+		randomValidator := validatorAddresses[rand.Intn(numValidators)]
+
+		err = keeper.bankKeeper.SendCoinsFromModuleToAccount(ctx, types.ModuleName, randomValidator, remainingAmount)
+		if err != nil {
+			keeper.Logger(ctx).Error("Failed to send remaining coins to a random validator", "validator", randomValidator, "error", err)
+			return err
+		}
+	}
+
+	keeper.Logger(ctx).Info("Successfully distributed and deleted deposits")
+
+	return err
+}
+
 // validateInitialDeposit validates if initial deposit is greater than or equal to the minimum
 // required at the time of proposal submission. This threshold amount is determined by
 // the deposit parameters. Returns nil on success, error otherwise.

x/gov/keeper/deposit_test.go

@@ -1,10 +1,13 @@
 package keeper_test
 
 import (
+	"context"
 	"fmt"
 	"math/big"
 	"testing"
 
+	"github.com/golang/mock/gomock"
+
 	"github.com/stretchr/testify/require"
 
 	"cosmossdk.io/collections"
@@ -16,6 +19,8 @@ import (
 	authtypes "github.com/cosmos/cosmos-sdk/x/auth/types"
 	disttypes "github.com/cosmos/cosmos-sdk/x/distribution/types"
 	v1 "github.com/cosmos/cosmos-sdk/x/gov/types/v1"
+
+	stakeTypes "github.com/0xPolygon/heimdall-v2/x/stake/types"
 )
 
 const (
@@ -461,3 +466,141 @@ func TestChargeDeposit(t *testing.T) {
 		}
 	}
 }
+
+func TestDistributeAndDeleteDeposits(t *testing.T) {
+	testcases := []struct {
+		name          string
+		numValidators uint64
+	}{
+		{
+			name:          "Single validator case",
+			numValidators: 1,
+		},
+		{
+			name:          "Equal distribution case",
+			numValidators: 2,
+		},
+		{
+			name:          "Edge case: Distribution of the remaining amount to a random validator",
+			numValidators: 3,
+		},
+	}
+
+	for _, tc := range testcases {
+		t.Run(tc.name, func(t *testing.T) {
+			govKeeper, authKeeper, bankKeeper, stakingKeeper, distKeeper, _, ctx := setupGovKeeper(t)
+			trackMockBalances(bankKeeper, distKeeper)
+
+			accAmt := sdkmath.NewIntFromBigInt(new(big.Int).Mul(big.NewInt(10), new(big.Int).Exp(big.NewInt(10), big.NewInt(18), nil)))
+			TestAddrs := simtestutil.AddTestAddrsIncremental(bankKeeper, ctx, 5, accAmt.Mul(sdkmath.NewInt(1)))
+			authKeeper.EXPECT().AddressCodec().Return(address.NewHexCodec()).AnyTimes()
+
+			var mockValidators []stakeTypes.Validator
+
+			switch {
+			case tc.numValidators == 1:
+				mockValidators = []stakeTypes.Validator{
+					{Signer: TestAddrs[2].String()},
+				}
+			case tc.numValidators == 2:
+				mockValidators = []stakeTypes.Validator{
+					{Signer: TestAddrs[2].String()},
+					{Signer: TestAddrs[3].String()},
+				}
+			case tc.numValidators == 3:
+				mockValidators = []stakeTypes.Validator{
+					{Signer: TestAddrs[2].String()},
+					{Signer: TestAddrs[3].String()},
+					{Signer: TestAddrs[4].String()},
+				}
+			}
+
+			stakingKeeper.EXPECT().IterateCurrentValidatorsAndApplyFn(gomock.Any(), gomock.Any()).DoAndReturn(
+				func(ctx context.Context, fn func(stakeTypes.Validator) bool) error {
+					for _, validator := range mockValidators {
+						if stop := fn(validator); stop {
+							break
+						}
+					}
+					return nil
+				},
+			).AnyTimes()
+
+			tp := TestProposal
+			proposal, err := govKeeper.SubmitProposal(ctx, tp, "", "title", "summary", TestAddrs[0], false)
+			require.NoError(t, err)
+			proposalID := proposal.Id
+
+			stakeAmount := sdk.NewCoins(sdk.NewCoin(sdk.DefaultBondDenom, stakingKeeper.TokensFromConsensusPower(ctx, 5)))
+
+			addr0Initial := bankKeeper.GetAllBalances(ctx, TestAddrs[0])
+			addr1Initial := bankKeeper.GetAllBalances(ctx, TestAddrs[1])
+
+			addr2Initial := bankKeeper.GetAllBalances(ctx, TestAddrs[2])
+			addr3Initial := bankKeeper.GetAllBalances(ctx, TestAddrs[3])
+			addr4Initial := bankKeeper.GetAllBalances(ctx, TestAddrs[4])
+
+			// 1st deposit from TestAddrs[0]
+			_, err = govKeeper.AddDeposit(ctx, proposalID, TestAddrs[0], stakeAmount)
+			require.NoError(t, err)
+
+			// 2nd deposit from TestAddrs[1]
+			_, err = govKeeper.AddDeposit(ctx, proposalID, TestAddrs[1], stakeAmount)
+			require.NoError(t, err)
+
+			// Check deposits length
+			deposits, _ := govKeeper.GetDeposits(ctx, proposalID)
+			require.Len(t, deposits, 2)
+
+			// Check TestAddrs[0] and TestAddrs[1] balances
+			require.Equal(t, addr0Initial.Sub(stakeAmount...), bankKeeper.GetAllBalances(ctx, TestAddrs[0]))
+			require.Equal(t, addr1Initial.Sub(stakeAmount...), bankKeeper.GetAllBalances(ctx, TestAddrs[1]))
+
+			addr0After := bankKeeper.GetAllBalances(ctx, TestAddrs[0])
+			addr1After := bankKeeper.GetAllBalances(ctx, TestAddrs[1])
+
+			// 10 pol deposited in total from TestAddrs[0] and TestAddrs[1]
+
+			// Test DistributeAndDeleteDeposits
+			err = govKeeper.DistributeAndDeleteDeposits(ctx, proposalID)
+			require.NoError(t, err)
+
+			// Check balances
+
+			// Balances of TestAddrs[0] and TestAddrs[1] should be the same
+			// as the deposits will be distributed to the validators
+			require.Equal(t, addr0After, bankKeeper.GetAllBalances(ctx, TestAddrs[0]))
+			require.Equal(t, addr1After, bankKeeper.GetAllBalances(ctx, TestAddrs[1]))
+
+			// Balances of validators will be dependent on numValidators
+			switch {
+			case tc.numValidators == 1:
+				// All the deposits will be transferred to the single validator
+				require.Equal(t, addr2Initial.Add(stakeAmount...).Add(stakeAmount...), bankKeeper.GetAllBalances(ctx, TestAddrs[2]))
+			case tc.numValidators == 2:
+				// Equal distribution of deposits among validators
+				require.Equal(t, addr2Initial.Add(stakeAmount...), bankKeeper.GetAllBalances(ctx, TestAddrs[2]))
+				require.Equal(t, addr3Initial.Add(stakeAmount...), bankKeeper.GetAllBalances(ctx, TestAddrs[3]))
+			case tc.numValidators == 3:
+				// A random validator will get a little bit of pol (NOT in the power 10**18) more because of truncation in division
+				addr2After := bankKeeper.GetAllBalances(ctx, TestAddrs[2])
+				addr3After := bankKeeper.GetAllBalances(ctx, TestAddrs[3])
+				addr4After := bankKeeper.GetAllBalances(ctx, TestAddrs[4])
+
+				if addr2After.AmountOf("pol").GT(addr3After.AmountOf("pol")) && addr2After.AmountOf("pol").GT(addr4After.AmountOf("pol")) {
+					require.Equal(t, addr2Initial.Add(sdk.NewCoin("pol", sdkmath.NewInt(3333333333333333336))), addr2After)
+					require.Equal(t, addr3Initial.Add(sdk.NewCoin("pol", sdkmath.NewInt(3333333333333333332))), addr3After)
+					require.Equal(t, addr4Initial.Add(sdk.NewCoin("pol", sdkmath.NewInt(3333333333333333332))), addr4After)
+				} else if addr3After.AmountOf("pol").GT(addr2After.AmountOf("pol")) && addr3After.AmountOf("pol").GT(addr4After.AmountOf("pol")) {
+					require.Equal(t, addr2Initial.Add(sdk.NewCoin("pol", sdkmath.NewInt(3333333333333333332))), addr2After)
+					require.Equal(t, addr3Initial.Add(sdk.NewCoin("pol", sdkmath.NewInt(3333333333333333336))), addr3After)
+					require.Equal(t, addr4Initial.Add(sdk.NewCoin("pol", sdkmath.NewInt(3333333333333333332))), addr4After)
+				} else {
+					require.Equal(t, addr2Initial.Add(sdk.NewCoin("pol", sdkmath.NewInt(3333333333333333332))), addr2After)
+					require.Equal(t, addr3Initial.Add(sdk.NewCoin("pol", sdkmath.NewInt(3333333333333333332))), addr3After)
+					require.Equal(t, addr4Initial.Add(sdk.NewCoin("pol", sdkmath.NewInt(3333333333333333336))), addr4After)
+				}
+			}
+		})
+	}
+}

x/gov/keeper/hooks_test.go

@@ -16,6 +16,8 @@ import (
 	"github.com/cosmos/cosmos-sdk/x/gov/keeper"
 	"github.com/cosmos/cosmos-sdk/x/gov/types"
 	v1 "github.com/cosmos/cosmos-sdk/x/gov/types/v1"
+
+	stakeTypes "github.com/0xPolygon/heimdall-v2/x/stake/types"
 )
 
 var _ types.GovHooks = &MockGovHooksReceiver{}
@@ -61,7 +63,22 @@ func TestHooks(t *testing.T) {
 
 	authKeeper.EXPECT().AddressCodec().Return(address.NewHexCodec()).AnyTimes()
 	stakingKeeper.EXPECT().ValidatorAddressCodec().Return(address.NewHexCodec()).AnyTimes()
-	stakingKeeper.EXPECT().IterateCurrentValidatorsAndApplyFn(gomock.Any(), gomock.Any()).AnyTimes()
+
+	mockValidators := []stakeTypes.Validator{
+		{Signer: "0xb316fa9fa91700d7084d377bfdc81eb9f232f5ff"},
+		{Signer: "0xb316fa9fa91700d7084d377bfdc81eb9f232f5fd"},
+	}
+
+	stakingKeeper.EXPECT().IterateCurrentValidatorsAndApplyFn(gomock.Any(), gomock.Any()).DoAndReturn(
+		func(ctx context.Context, fn func(stakeTypes.Validator) bool) error {
+			for _, validator := range mockValidators {
+				if stop := fn(validator); stop {
+					break
+				}
+			}
+			return nil
+		},
+	).AnyTimes()
 
 	govHooksReceiver := MockGovHooksReceiver{}
 

x/gov/testutil/expected_keepers.go

@@ -3,7 +3,7 @@
 package testutil
 
 import (
-	context "context"
+	"context"
 
 	"cosmossdk.io/math"