fix(x/gov): limit execution in gov (#20348)

Co-authored-by: Alexander Peters <[email protected]>

x/accounts/go.mod

@@ -8,6 +8,7 @@ require (
 	cosmossdk.io/core v0.12.1-0.20231114100755-569e3ff6a0d7
 	cosmossdk.io/core/testing v0.0.0-00010101000000-000000000000
 	cosmossdk.io/depinject v1.0.0-alpha.4
+	cosmossdk.io/x/bank v0.0.0-20240226161501-23359a0b6d91
 	cosmossdk.io/x/tx v0.13.3
 	github.com/cosmos/cosmos-sdk v0.51.0
 	github.com/cosmos/gogoproto v1.5.0
@@ -27,7 +28,6 @@ require (
 	cosmossdk.io/store v1.1.1-0.20240418092142-896cdf1971bc // indirect
 	cosmossdk.io/x/accounts/defaults/lockup v0.0.0-20240417181816-5e7aae0db1f5
 	cosmossdk.io/x/auth v0.0.0-00010101000000-000000000000 // indirect
-	cosmossdk.io/x/bank v0.0.0-20240226161501-23359a0b6d91
 	cosmossdk.io/x/consensus v0.0.0-00010101000000-000000000000 // indirect
 	cosmossdk.io/x/staking v0.0.0-00010101000000-000000000000 // indirect
 	filippo.io/edwards25519 v1.1.0 // indirect

x/gov/CHANGELOG.md

@@ -56,6 +56,7 @@ Ref: https://keepachangelog.com/en/1.0.0/
 * [#18762](https://github.com/cosmos/cosmos-sdk/pull/18762) Add multiple choice proposals.
 * [#18856](https://github.com/cosmos/cosmos-sdk/pull/18856) Add `ProposalCancelMaxPeriod` parameters.
 * [#19167](https://github.com/cosmos/cosmos-sdk/pull/19167) Add `YesQuorum` parameter.
+* [#20348](https://github.com/cosmos/cosmos-sdk/pull/20348) Limit gov execution of proposals to a max gas limit. The limit was added to parameters and can be modified. With this version the default is set to 10 million gas. Before it was infinite gas.
 
 ### Client Breaking Changes
 

x/gov/README.md

@@ -24,7 +24,7 @@ they don't vote themselves.
 deposits if the proposal was accepted or rejected. If the proposal was vetoed, or never entered voting period (minimum deposit not reached within deposit period), the deposit is burned.
 
 This module is in use on the Cosmos Hub (a.k.a [gaia](https://github.com/cosmos/gaia)).
-Features that may be added in the future are described in [Future Improvements](#future-improvements).
+
 
 ## Contents
 
@@ -278,6 +278,12 @@ There are three parameters that define if the deposit of a proposal should be bu
 
 > Note: These parameters are modifiable via governance.
 
+#### Execution
+
+Execution is the process of executing the messages contained in a proposal. The execution phase will commence after the proposal has been accepted by the network. The messages contained in the proposal will be executed in the order they were submitted.
+
+Execution has a upper limit on how much gas can be consumed in a single block. This limit is defined by the `ProposalExecutionGas` parameter.
+
 ## State
 
 ### Constitution

x/gov/go.mod

@@ -13,7 +13,9 @@ require (
 	cosmossdk.io/math v1.3.0
 	cosmossdk.io/store v1.1.1-0.20240418092142-896cdf1971bc
 	cosmossdk.io/x/accounts v0.0.0-20240226161501-23359a0b6d91 // indirect
+	cosmossdk.io/x/auth v0.0.0-00010101000000-000000000000
 	cosmossdk.io/x/bank v0.0.0-20240226161501-23359a0b6d91
+	cosmossdk.io/x/consensus v0.0.0-00010101000000-000000000000 // indirect
 	cosmossdk.io/x/protocolpool v0.0.0-20230925135524-a1bc045b3190
 	cosmossdk.io/x/staking v0.0.0-00010101000000-000000000000
 	github.com/chzyer/readline v1.5.1
@@ -39,8 +41,6 @@ require (
 require (
 	buf.build/gen/go/cometbft/cometbft/protocolbuffers/go v1.34.1-20240312114316-c0d3497e35d6.1 // indirect
 	cosmossdk.io/x/accounts/defaults/lockup v0.0.0-20240417181816-5e7aae0db1f5 // indirect
-	cosmossdk.io/x/auth v0.0.0-00010101000000-000000000000
-	cosmossdk.io/x/consensus v0.0.0-00010101000000-000000000000 // indirect
 	cosmossdk.io/x/tx v0.13.3 // indirect
 	filippo.io/edwards25519 v1.1.0 // indirect
 	github.com/99designs/go-keychain v0.0.0-20191008050251-8e49817e8af4 // indirect

x/gov/keeper/abci.go

@@ -35,6 +35,11 @@ func (k Keeper) EndBlocker(ctx context.Context) error {
 		return err
 	}
 
+	params, err := k.Params.Get(ctx)
+	if err != nil {
+		return err
+	}
+
 	for _, prop := range inactiveProps {
 		proposal, err := k.Proposals.Get(ctx, prop.Key.K2())
 		if err != nil {
@@ -61,10 +66,6 @@ func (k Keeper) EndBlocker(ctx context.Context) error {
 			return err
 		}
 
-		params, err := k.Params.Get(ctx)
-		if err != nil {
-			return err
-		}
 		if !params.BurnProposalDepositPrevote {
 			err = k.RefundAndDeleteDeposits(ctx, proposal.Id) // refund deposit if proposal got removed without getting 100% of the proposal
 		} else {
@@ -77,7 +78,7 @@ func (k Keeper) EndBlocker(ctx context.Context) error {
 
 		// called when proposal become inactive
 		// call hook when proposal become inactive
-		if err := k.BranchService.Execute(ctx, func(ctx context.Context) error {
+		if err = k.BranchService.Execute(ctx, func(ctx context.Context) error {
 			return k.Hooks().AfterProposalFailedMinDeposit(ctx, proposal.Id)
 		}); err != nil {
 			// purposely ignoring the error here not to halt the chain if the hook fails
@@ -193,16 +194,10 @@ func (k Keeper) EndBlocker(ctx context.Context) error {
 			// Messages may mutate state thus we use a cached context. If one of
 			// the handlers fails, no state mutation is written and the error
 			// message is logged.
-			if err := k.BranchService.Execute(ctx, func(ctx context.Context) error {
+			_, err = k.BranchService.ExecuteWithGasLimit(ctx, params.ProposalExecutionGas, func(ctx context.Context) error {
 				// execute all messages
 				for idx, msg = range messages {
 					if _, err := safeExecuteHandler(ctx, msg, k.MsgRouterService); err != nil {
-						// `idx` and `err` are populated with the msg index and error.
-						proposal.Status = v1.StatusFailed
-						proposal.FailedReason = err.Error()
-						tagValue = types.AttributeValueProposalFailed
-						logMsg = fmt.Sprintf("passed, but msg %d (%s) failed on execution: %s", idx, sdk.MsgTypeURL(msg), err)
-
 						return err
 					}
 				}
@@ -212,7 +207,14 @@ func (k Keeper) EndBlocker(ctx context.Context) error {
 				logMsg = "passed"
 
 				return nil
-			}); err != nil {
+			})
+			if err != nil {
+				// `idx` and `err` are populated with the msg index and error.
+				proposal.Status = v1.StatusFailed
+				proposal.FailedReason = err.Error()
+				tagValue = types.AttributeValueProposalFailed
+				logMsg = fmt.Sprintf("passed, but msg %d (%s) failed on execution: %s", idx, sdk.MsgTypeURL(msg), err)
+
 				break // We do not anything with the error. Returning an error halts the chain, and proposal struct is already updated.
 			}
 		case !burnDeposits && (proposal.ProposalType == v1.ProposalType_PROPOSAL_TYPE_EXPEDITED ||
@@ -222,10 +224,6 @@ func (k Keeper) EndBlocker(ctx context.Context) error {
 			// once the regular voting period expires again, the tally is repeated
 			// according to the regular proposal rules.
 			proposal.ProposalType = v1.ProposalType_PROPOSAL_TYPE_STANDARD
-			params, err := k.Params.Get(ctx)
-			if err != nil {
-				return err
-			}
 			endTime := proposal.VotingStartTime.Add(*params.VotingPeriod)
 			proposal.VotingEndTime = &endTime
 

x/gov/migrations/v6/store.go

@@ -11,7 +11,7 @@ import (
 
 var votingPeriodProposalKeyPrefix = collections.NewPrefix(4) // VotingPeriodProposalKeyPrefix stores which proposals are on voting period.
 
-// MigrateStore performs in-place store migrations from v5 (v0.50) to v6 (v0.51). The
+// MigrateStore performs in-place store migrations from v5 (v0.50) to v6 (v0.52). The
 // migration includes:
 //
 // Addition of new field in params to store types of proposals that can be submitted.
@@ -56,6 +56,7 @@ func MigrateStore(ctx context.Context, storeService corestoretypes.KVStoreServic
 	govParams.OptimisticAuthorizedAddresses = defaultParams.OptimisticAuthorizedAddresses
 	govParams.OptimisticRejectedThreshold = defaultParams.OptimisticRejectedThreshold
 	govParams.ProposalCancelMaxPeriod = defaultParams.ProposalCancelMaxPeriod
+	govParams.ProposalExecutionGas = defaultParams.ProposalExecutionGas
 
 	return paramsCollection.Set(ctx, govParams)
 }

x/gov/proto/cosmos/gov/v1/gov.proto

@@ -342,7 +342,9 @@ message Params {
 
   // Minimum percentage of total stake needed to vote for a result to be
   // considered valid for an expedited proposal.
-  string expedited_quorum = 21 [(cosmos_proto.scalar) = "cosmos.Dec", (cosmos_proto.field_added_in) = "x/gov v0.2.0"];
+  string expedited_quorum = 21 [(cosmos_proto.scalar) = "cosmos.Dec", (cosmos_proto.field_added_in) = "x/gov v1.0.0"];
+
+  uint64 proposal_execution_gas = 22 [(cosmos_proto.field_added_in) = "x/gov v0.2.0"];
 }
 
 // MessageBasedParams defines the parameters of specific messages in a proposal.
@@ -365,4 +367,4 @@ message MessageBasedParams {
 
   // Minimum value of Veto votes to Total votes ratio for proposal to be vetoed.
   string veto_threshold = 4 [(cosmos_proto.scalar) = "cosmos.Dec"];
-}
+}

x/gov/simulation/genesis.go

@@ -194,6 +194,7 @@ func RandomizedGenState(simState *module.SimulationState) {
 			minDepositRatio.String(),
 			optimisticRejectedThreshold.String(),
 			[]string{},
+			10_000_000,
 		),
 	)