feat(x/gov): add custom fuction to calculate vote results and vp (cos…

…mos#19592)

docs/architecture/adr-069-gov-improvements.md

@@ -164,19 +164,15 @@ Due to the vote option change, each proposal can have the same tallying method.
 
 However, chains may want to change the tallying function (weighted vote per voting power) of `x/gov` for a different algorithm (using a quadratic function on the voter stake, for instance).
 
-The custom tallying function can be passed to the `x/gov` keeper with the following interface:
+The custom tallying function can be passed to the `x/gov` keeper config:
 
 ```go
-type Tally interface{
-    // to be decided
-
-    // Calculate calculates the tally result
-    Calculate(proposal v1.Proposal, govKeeper GovKeeper, stakingKeeper StakingKeeper) govv1.TallyResult
-    // IsAccepted returns true if the proposal passes/is accepted
-    IsAccepted() bool
-    // BurnDeposit returns true if the proposal deposit should be burned
-    BurnDeposit() bool
-}
+type CalculateVoteResultsAndVotingPowerFn func(
+	ctx context.Context,
+	keeper Keeper,
+	proposalID uint64,
+	validators map[string]v1.ValidatorGovInfo,
+) (totalVoterPower math.LegacyDec, results map[v1.VoteOption]math.LegacyDec, err error)
 ```
 
 ## Consequences

simapp/app.go

@@ -375,7 +375,7 @@ func NewSimApp(
 	// by granting the governance module the right to execute the message.
 	// See: https://docs.cosmos.network/main/modules/gov#proposal-messages
 	govRouter := govv1beta1.NewRouter()
-	govConfig := govtypes.DefaultConfig()
+	govConfig := govkeeper.DefaultConfig()
 	/*
 		Example of setting gov params:
 		govConfig.MaxMetadataLen = 10000

tests/integration/gov/keeper/keeper_test.go

@@ -112,7 +112,7 @@ func initFixture(tb testing.TB) *fixture {
 		stakingKeeper,
 		poolKeeper,
 		router,
-		types.DefaultConfig(),
+		keeper.DefaultConfig(),
 		authority.String(),
 	)
 	assert.NilError(tb, govKeeper.ProposalID.Set(newCtx, 1))

x/gov/CHANGELOG.md

@@ -27,6 +27,7 @@ Ref: https://keepachangelog.com/en/1.0.0/
 
 ### Features
 
+* [#19592](https://github.com/cosmos/cosmos-sdk/pull/19592) Add custom tally function.
 * [#19304](https://github.com/cosmos/cosmos-sdk/pull/19304) Add `MsgSudoExec` for allowing executing any message as a sudo.
 * [#19101](https://github.com/cosmos/cosmos-sdk/pull/19101) Add message based params configuration.
 * [#18532](https://github.com/cosmos/cosmos-sdk/pull/18532) Add SPAM vote to proposals.
@@ -59,6 +60,7 @@ Ref: https://keepachangelog.com/en/1.0.0/
 
 ### API Breaking Changes
 
+* [#19592](https://github.com/cosmos/cosmos-sdk/pull/19592) `types.Config` and `types.DefaultConfig` have been moved to the keeper package in order to support the custom tallying function.
 * [#19349](https://github.com/cosmos/cosmos-sdk/pull/19349) Simplify state management in `x/gov`. Note `k.VotingPeriodProposals` and `k.SetProposal` are no longer needed and have been removed.
 * [#18532](https://github.com/cosmos/cosmos-sdk/pull/18532) All functions that were taking an expedited bool parameter now take a `ProposalType` parameter instead.
 * [#17496](https://github.com/cosmos/cosmos-sdk/pull/17496) in `x/gov/types/v1beta1/vote.go` `NewVote` was removed, constructing the struct is required for this type.

x/gov/depinject.go

@@ -60,7 +60,7 @@ type ModuleOutputs struct {
 }
 
 func ProvideModule(in ModuleInputs) ModuleOutputs {
-	defaultConfig := govtypes.DefaultConfig()
+	defaultConfig := keeper.DefaultConfig()
 	if in.Config.MaxTitleLen != 0 {
 		defaultConfig.MaxTitleLen = in.Config.MaxTitleLen
 	}

x/gov/keeper/common_test.go

@@ -130,7 +130,7 @@ func setupGovKeeper(t *testing.T, expectations ...func(sdk.Context, mocks)) (
 
 	// Gov keeper initializations
 
-	govKeeper := keeper.NewKeeper(encCfg.Codec, storeService, m.acctKeeper, m.bankKeeper, m.stakingKeeper, m.poolKeeper, baseApp.MsgServiceRouter(), types.DefaultConfig(), govAcct.String())
+	govKeeper := keeper.NewKeeper(encCfg.Codec, storeService, m.acctKeeper, m.bankKeeper, m.stakingKeeper, m.poolKeeper, baseApp.MsgServiceRouter(), keeper.DefaultConfig(), govAcct.String())
 	require.NoError(t, govKeeper.ProposalID.Set(ctx, 1))
 	govRouter := v1beta1.NewRouter() // Also register legacy gov handlers to test them too.
 	govRouter.AddRoute(types.RouterKey, v1beta1.ProposalHandler)

x/gov/keeper/config.go

@@ -0,0 +1,42 @@
+package keeper
+
+import (
+	"context"
+
+	"cosmossdk.io/math"
+	v1 "cosmossdk.io/x/gov/types/v1"
+)
+
+// CalculateVoteResultsAndVotingPowerFn is a function signature for calculating vote results and voting power
+// It can be overridden to customize the voting power calculation for proposals
+// It gets the proposal tallied and the validators governance infos (bonded tokens, voting power, etc.)
+// It must return the total voting power and the results of the vote
+type CalculateVoteResultsAndVotingPowerFn func(
+	ctx context.Context,
+	keeper Keeper,
+	proposalID uint64,
+	validators map[string]v1.ValidatorGovInfo,
+) (totalVoterPower math.LegacyDec, results map[v1.VoteOption]math.LegacyDec, err error)
+
+// Config is a config struct used for initializing the gov module to avoid using globals.
+type Config struct {
+	// MaxTitleLen defines the amount of characters that can be used for proposal title
+	MaxTitleLen uint64
+	// MaxMetadataLen defines the amount of characters that can be used for proposal metadata
+	MaxMetadataLen uint64
+	// MaxSummaryLen defines the amount of characters that can be used for proposal summary
+	MaxSummaryLen uint64
+	// CalculateVoteResultsAndVotingPowerFn is a function signature for calculating vote results and voting power
+	// Keeping it nil will use the default implementation
+	CalculateVoteResultsAndVotingPowerFn CalculateVoteResultsAndVotingPowerFn
+}
+
+// DefaultConfig returns the default config for gov.
+func DefaultConfig() Config {
+	return Config{
+		MaxTitleLen:                          255,
+		MaxMetadataLen:                       255,
+		MaxSummaryLen:                        10200,
+		CalculateVoteResultsAndVotingPowerFn: nil,
+	}
+}

x/gov/keeper/deposit.go

@@ -98,7 +98,7 @@ func (k Keeper) AddDeposit(ctx context.Context, proposalID uint64, depositorAddr
 	}
 
 	// the deposit must only contain valid denoms (listed in the min deposit param)
-	if err := k.validateDepositDenom(ctx, params, depositAmount); err != nil {
+	if err := k.validateDepositDenom(params, depositAmount); err != nil {
 		return false, err
 	}
 
@@ -280,7 +280,7 @@ func (k Keeper) ChargeDeposit(ctx context.Context, proposalID uint64, destAddres
 // 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.
-func (k Keeper) validateInitialDeposit(ctx context.Context, params v1.Params, initialDeposit sdk.Coins, proposalType v1.ProposalType) error {
+func (k Keeper) validateInitialDeposit(params v1.Params, initialDeposit sdk.Coins, proposalType v1.ProposalType) error {
 	if !initialDeposit.IsValid() || initialDeposit.IsAnyNegative() {
 		return errors.Wrap(sdkerrors.ErrInvalidCoins, initialDeposit.String())
 	}
@@ -311,7 +311,7 @@ func (k Keeper) validateInitialDeposit(ctx context.Context, params v1.Params, in
 }
 
 // validateDepositDenom validates if the deposit denom is accepted by the governance module.
-func (k Keeper) validateDepositDenom(ctx context.Context, params v1.Params, depositAmount sdk.Coins) error {
+func (k Keeper) validateDepositDenom(params v1.Params, depositAmount sdk.Coins) error {
 	denoms := []string{}
 	acceptedDenoms := make(map[string]bool, len(params.MinDeposit))
 	for _, coin := range params.MinDeposit {

x/gov/keeper/export_test.go

@@ -14,5 +14,5 @@ func (k Keeper) ValidateInitialDeposit(ctx sdk.Context, initialDeposit sdk.Coins
 		return err
 	}
 
-	return k.validateInitialDeposit(ctx, params, initialDeposit, proposalType)
+	return k.validateInitialDeposit(params, initialDeposit, proposalType)
 }

x/gov/keeper/keeper.go

@@ -42,7 +42,8 @@ type Keeper struct {
 	// Msg server router
 	router baseapp.MessageRouter
 
-	config types.Config
+	// Config represent extra module configuration
+	config Config
 
 	// the address capable of executing a MsgUpdateParams message. Typically, this
 	// should be the x/gov module account.
@@ -88,7 +89,7 @@ func (k Keeper) GetAuthority() string {
 func NewKeeper(
 	cdc codec.Codec, storeService corestoretypes.KVStoreService, authKeeper types.AccountKeeper,
 	bankKeeper types.BankKeeper, sk types.StakingKeeper, pk types.PoolKeeper,
-	router baseapp.MessageRouter, config types.Config, authority string,
+	router baseapp.MessageRouter, config Config, authority string,
 ) *Keeper {
 	// ensure governance module account is set
 	if addr := authKeeper.GetModuleAddress(types.ModuleName); addr == nil {
@@ -99,7 +100,7 @@ func NewKeeper(
 		panic(fmt.Sprintf("invalid authority address: %s", authority))
 	}
 
-	defaultConfig := types.DefaultConfig()
+	defaultConfig := DefaultConfig()
 	// If MaxMetadataLen not set by app developer, set to default value.
 	if config.MaxTitleLen == 0 {
 		config.MaxTitleLen = defaultConfig.MaxTitleLen

x/gov/keeper/msg_server.go

@@ -76,11 +76,11 @@ func (k msgServer) SubmitProposal(goCtx context.Context, msg *v1.MsgSubmitPropos
 	if msg.Expedited { // checking for backward compatibility
 		msg.ProposalType = v1.ProposalType_PROPOSAL_TYPE_EXPEDITED
 	}
-	if err := k.validateInitialDeposit(ctx, params, msg.GetInitialDeposit(), msg.ProposalType); err != nil {
+	if err := k.validateInitialDeposit(params, msg.GetInitialDeposit(), msg.ProposalType); err != nil {
 		return nil, err
 	}
 
-	if err := k.validateDepositDenom(ctx, params, msg.GetInitialDeposit()); err != nil {
+	if err := k.validateDepositDenom(params, msg.GetInitialDeposit()); err != nil {
 		return nil, err
 	}
 

x/gov/keeper/proposal.go

@@ -264,7 +264,7 @@ func (k Keeper) ActivateVotingPeriod(ctx context.Context, proposal v1.Proposal)
 			customMessageParams, err := k.MessageBasedParams.Get(ctx, sdk.MsgTypeURL(proposal.Messages[0]))
 			if err == nil {
 				votingPeriod = customMessageParams.VotingPeriod
-			} else if err != nil && !errors.Is(err, collections.ErrNotFound) {
+			} else if !errors.Is(err, collections.ErrNotFound) {
 				return err
 			}
 		}

x/gov/keeper/tally.go

@@ -18,7 +18,11 @@ func (k Keeper) Tally(ctx context.Context, proposal v1.Proposal) (passes, burnDe
 		return false, false, v1.TallyResult{}, err
 	}
 
-	totalVoterPower, results, err := k.calculateVoteResultsAndVotingPower(ctx, proposal.Id, validators)
+	if k.config.CalculateVoteResultsAndVotingPowerFn == nil {
+		k.config.CalculateVoteResultsAndVotingPowerFn = defaultCalculateVoteResultsAndVotingPower
+	}
+
+	totalVoterPower, results, err := k.config.CalculateVoteResultsAndVotingPowerFn(ctx, k, proposal.Id, validators)
 	if err != nil {
 		return false, false, v1.TallyResult{}, err
 	}
@@ -232,8 +236,9 @@ func (k Keeper) getCurrentValidators(ctx context.Context) (map[string]v1.Validat
 
 // calculateVoteResultsAndVotingPower iterate over all votes, tally up the voting power of each validator
 // and returns the votes results from voters
-func (k Keeper) calculateVoteResultsAndVotingPower(
+func defaultCalculateVoteResultsAndVotingPower(
 	ctx context.Context,
+	k Keeper,
 	proposalID uint64,
 	validators map[string]v1.ValidatorGovInfo,
 ) (math.LegacyDec, map[v1.VoteOption]math.LegacyDec, error) {