fix(x/gov): limit execution in gov #20348
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -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/tx v0.13.3 // indirect | ||
| filippo.io/edwards25519 v1.1.0 // indirect | ||
| github.com/99designs/go-keychain v0.0.0-20191008050251-8e49817e8af4 // indirect | ||
|
|
||
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -16,6 +16,7 @@ import ( | |
|
|
||
| "github.com/cosmos/cosmos-sdk/telemetry" | ||
| sdk "github.com/cosmos/cosmos-sdk/types" | ||
| sdkerrors "github.com/cosmos/cosmos-sdk/types/errors" | ||
| ) | ||
|
|
||
| // EndBlocker is called every block. | ||
|
|
@@ -35,6 +36,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 +67,6 @@ func (k Keeper) EndBlocker(ctx context.Context) error { | |
| 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 +79,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 { | ||
| 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 +195,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. | ||
| _, 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 { | ||
| return err | ||
| } | ||
| } | ||
|
|
@@ -212,7 +208,21 @@ func (k Keeper) EndBlocker(ctx context.Context) error { | |
| logMsg = "passed" | ||
|
|
||
| return nil | ||
| }) | ||
| if err != nil { | ||
| // update proposal if error is out of gas | ||
| if errors.Is(err, sdkerrors.ErrOutOfGas) { | ||
|
There was a problem hiding this comment. problem is branch service implementation of stf doesn't return this error There was a problem hiding this comment. should it be in all cases we set the proposal to fail? if not then we need to see how to make this work correctly There was a problem hiding this comment. yes, we can just remove this https://github.com/cosmos/cosmos-sdk/pull/20348/files/12ad1f9e4f4600950f089e358215d5fc68bcae6d#diff-4bb7b38066b637ade8ae58c5d5bf4050e08f2af0f89aa791baee39cb008187b9R206-R212 (but store the proposal idx), and handle for all error here instead. There was a problem hiding this comment. pushed what i think you mean, not sure where to store the proposal idx There was a problem hiding this comment. we need to remove L214-L219 and make the error log more generic (as it can be out of gas or proposal execution error (at handling)). As it is only used in the log, let's skip it? |
||
| proposal.Status = v1.StatusFailed | ||
| proposal.FailedReason = err.Error() | ||
| tagValue = types.AttributeValueProposalFailed | ||
| logMsg = "passed proposal failed due to out of gas" | ||
|
There was a problem hiding this comment. There is a break missing or this is overwritten in L224. Is there a way for people to find out the required gas amount? If the original error msg contains this information it feels like the better choice. There was a problem hiding this comment. That whole if condition above should be removed (L214-L219) |
||
| } | ||
| // `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 +232,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 | ||
| endTime := proposal.VotingStartTime.Add(*params.VotingPeriod) | ||
| proposal.VotingEndTime = &endTime | ||
|
|
||
|
|
||
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -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.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) | ||
| } | ||
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -343,6 +343,8 @@ 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 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"]; | ||
| } | ||
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -194,6 +194,7 @@ func RandomizedGenState(simState *module.SimulationState) { | |
| minDepositRatio.String(), | ||
| optimisticRejectedThreshold.String(), | ||
| []string{}, | ||
| 10_000_000, | ||
| ), | ||
| ) | ||
|
|
||
|
|
||
Oops, something went wrong.
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Add a comma for better readability.
Committable suggestion
Tools
LanguageTool
Correct the pluralization for grammatical accuracy.
Committable suggestion
Tools
LanguageTool
Refine the changelog entry for better clarity and correctness.
Committable suggestion
Tools
LanguageTool