fix(x/gov): limit execution in gov #20348
Conversation
WalkthroughWalkthroughThis update introduces a new requirement for the Changes
Sequence DiagramssequenceDiagram
participant User
participant GovernanceModule
participant Proposal
participant Context
User->>GovernanceModule: Submit Proposal
GovernanceModule->>Proposal: Validate Proposal
Proposal->>GovernanceModule: Proposal Validated
GovernanceModule->>Context: Execute Proposal
Context-->>GovernanceModule: Out-of-Gas Error
GovernanceModule->>User: Proposal Execution Failed
sequenceDiagram
participant Developer
participant MigrationScript
participant GovernanceModule
Developer->>MigrationScript: Run Migration to v6
MigrationScript->>GovernanceModule: Add ProposalExecutionGas to govParams
GovernanceModule->>MigrationScript: Migration Successful
MigrationScript->>Developer: Migration Completed
Warning Review ran into problemsProblems (1)
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (invoked as PR comments)
Additionally, you can add CodeRabbit Configration File (
|
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
x/gov/README.md
Outdated
|
|
||
| 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 many messages can be executed in a single block. This limit is defined by the `MaxMessagesPerProposal` parameter. We use the MaxBlockGas from the consensus engine, value stored in the consensus module, to calculate the limit of messages that can be executed in a block. |
There was a problem hiding this comment.
Why do we need a MaxMessagesPerProposal if we already have that block gas limit?
x/gov/keeper/abci.go
Outdated
| _, 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 | ||
| } | ||
| } | ||
|
|
||
| proposal.Status = v1.StatusPassed | ||
| tagValue = types.AttributeValueProposalPassed | ||
| logMsg = "passed" | ||
|
|
||
| return nil | ||
| }); err != nil { | ||
| }) |
Check warning
Code scanning / CodeQL
Panic in BeginBock or EndBlock consensus methods Warning
|
@tac0turtle your pull request is missing a changelog! |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
Outside diff range and nitpick comments (7)
x/gov/README.md (7)
Line range hint
26-26: Add a period after "a.k.a" for correct abbreviation usage.- This module is in use on the Cosmos Hub (a.k.a [gaia](https://github.com/cosmos/gaia)). + This module is in use on the Cosmos Hub (a.k.a. [gaia](https://github.com/cosmos/gaia)).Tools
LanguageTool
[misspelling] ~285-~285: Use “an” instead of ‘a’ if the following word starts with a vowel sound, e.g. ‘an article’, ‘an hour’.
Context: ...der they were submitted. Execution has a upper limit on how much gas can be cons...
Line range hint
94-94: Replace "ie." with "i.e." to correct the abbreviation usage.- Messages without authority are message meant to be executed by users. Using the `MsgSudoExec` message in a proposal, let governance can execute any message, effectively acting as super user. + Messages without authority are message meant to be executed by users. Using the `MsgSudoExec` message in a proposal, let governance can execute any message, effectively acting as super user (i.e., no authority present).Tools
LanguageTool
[misspelling] ~285-~285: Use “an” instead of ‘a’ if the following word starts with a vowel sound, e.g. ‘an article’, ‘an hour’.
Context: ...der they were submitted. Execution has a upper limit on how much gas can be cons...
Line range hint
95-95: Change "super user" to "superuser" for correct terminology.- Messages without authority are message meant to be executed by users. Using the `MsgSudoExec` message in a proposal, let governance can execute any message, effectively acting as super user. + Messages without authority are message meant to be executed by users. Using the `MsgSudoExec` message in a proposal, let governance can execute any message, effectively acting as superuser.Tools
LanguageTool
[misspelling] ~285-~285: Use “an” instead of ‘a’ if the following word starts with a vowel sound, e.g. ‘an article’, ‘an hour’.
Context: ...der they were submitted. Execution has a upper limit on how much gas can be cons...
Line range hint
103-103: Change "accompanied with" to "accompanied by" for correct preposition usage.- When a proposal is submitted, it has to be accompanied with a deposit that must be strictly positive... + When a proposal is submitted, it has to be accompanied by a deposit that must be strictly positive...Tools
LanguageTool
[misspelling] ~285-~285: Use “an” instead of ‘a’ if the following word starts with a vowel sound, e.g. ‘an article’, ‘an hour’.
Context: ...der they were submitted. Execution has a upper limit on how much gas can be cons...
Line range hint
110-110: Add "the" before "state" for correct article usage.- ...the proposal will be removed from state and the deposit will be burned... + ...the proposal will be removed from the state and the deposit will be burned...Tools
LanguageTool
[misspelling] ~285-~285: Use “an” instead of ‘a’ if the following word starts with a vowel sound, e.g. ‘an article’, ‘an hour’.
Context: ...der they were submitted. Execution has a upper limit on how much gas can be cons...
Line range hint
136-136: Replace "that" with "who" when referring to people for correct pronoun usage.- *Participants* are users that have the right to vote on proposals. + *Participants* are users who have the right to vote on proposals.Tools
LanguageTool
[misspelling] ~285-~285: Use “an” instead of ‘a’ if the following word starts with a vowel sound, e.g. ‘an article’, ‘an hour’.
Context: ...der they were submitted. Execution has a upper limit on how much gas can be cons...
Line range hint
172-172: Add a comma after "No" for better readability.- Often times the entity owning that address might not be a single individual. For example, a company might have different stakeholders who want to vote differently, and so it makes sense to allow them to split their voting power. Currently, it is not possible for them to do "passthrough voting" and giving their users voting rights over their tokens. However, with this system, exchanges can poll their users for voting preferences, and then vote on-chain proportionally to the results of the poll. + Often times the entity owning that address might not be a single individual. For example, a company might have different stakeholders who want to vote differently, and so it makes sense to allow them to split their voting power. Currently, it is not possible for them to do "passthrough voting" and giving their users voting rights over their tokens. However, with this system, exchanges can poll their users for voting preferences, and then vote on-chain proportionally to the results of the poll. Often times the entity owning that address might not be a single individual. For example, a company might have different stakeholders who want to vote differently, and so it makes sense to allow them to split their voting power. Currently, it is not possible for them to do "passthrough voting" and giving their users voting rights over their tokens. However, with this system, exchanges can poll their users for voting preferences, and then vote on-chain proportionally to the results of the poll.Tools
LanguageTool
[misspelling] ~285-~285: Use “an” instead of ‘a’ if the following word starts with a vowel sound, e.g. ‘an article’, ‘an hour’.
Context: ...der they were submitted. Execution has a upper limit on how much gas can be cons...
Review details
Configuration used: .coderabbit.yml
Review profile: CHILL
Files ignored due to path filters (2)
x/gov/go.modis excluded by!**/*.modx/gov/types/v1/gov.pb.gois excluded by!**/*.pb.go
Files selected for processing (7)
- api/cosmos/gov/v1/gov.pulsar.go (16 hunks)
- x/gov/README.md (1 hunks)
- x/gov/keeper/abci.go (3 hunks)
- x/gov/migrations/v6/store.go (2 hunks)
- x/gov/proto/cosmos/gov/v1/gov.proto (2 hunks)
- x/gov/simulation/genesis.go (1 hunks)
- x/gov/types/v1/params.go (4 hunks)
Files not summarized due to errors (1)
- api/cosmos/gov/v1/gov.pulsar.go: Error: Message exceeds token limit
Additional context used
Path-based instructions (6)
x/gov/migrations/v6/store.go (1)
Pattern
**/*.go: Review the Golang code for conformity with the Uber Golang style guide, highlighting any deviations.x/gov/simulation/genesis.go (1)
Pattern
**/*.go: Review the Golang code for conformity with the Uber Golang style guide, highlighting any deviations.x/gov/keeper/abci.go (1)
Pattern
**/*.go: Review the Golang code for conformity with the Uber Golang style guide, highlighting any deviations.x/gov/types/v1/params.go (1)
Pattern
**/*.go: Review the Golang code for conformity with the Uber Golang style guide, highlighting any deviations.x/gov/README.md (1)
Pattern
**/*.md: "Assess the documentation for misspellings, grammatical errors, missing documentation and correctness"api/cosmos/gov/v1/gov.pulsar.go (1)
Pattern
**/*.go: Review the Golang code for conformity with the Uber Golang style guide, highlighting any deviations.
LanguageTool
x/gov/README.md
[uncategorized] ~26-~26: The abbreviation/initialism is missing a period after the last letter.
Context: ...his module is in use on the Cosmos Hub (a.k.a gaia)...
[grammar] ~87-~87: Possible subject-verb agreement error detected.
Context: ...self. Modules such asx/upgrade, that want to allow certain messages to be execute...
[style] ~94-~94: As a shorter alternative for ‘able to’, consider using “can”.
Context: ...eck. :::warning Ultimately, governance is able to execute any proposal, even if they were...
[uncategorized] ~94-~94: The abbreviation “i.e.” (= that is) requires two periods.
Context: ...n't meant to be executed by governance (ie. no authority present). Messages without...
[grammar] ~95-~95: This is normally spelled as one word.
Context: ...cute any message, effectively acting as super user. ::: ### Deposit To prevent spam, pro...
[grammar] ~103-~103: The usual collocation for “accompany” is “by”, not “with”.
Context: ...n a proposal is submitted, it has to be accompanied with a deposit that must be strictly positiv...
[uncategorized] ~110-~110: Possible missing article found.
Context: ...oyed: the proposal will be removed from state and the deposit will be burned (see x/g...
[style] ~136-~136: Consider using “who” when you are referring to people instead of objects.
Context: ... Participants Participants are users that have the right to vote on proposals. On...
[uncategorized] ~172-~172: Possible missing comma found.
Context: ... of its voting power to vote No. Often times the entity owning that address might no...
[typographical] ~198-~198: It seems that a comma is missing.
Context: ...oposal for the result to be valid. ### Yes Quorum Yes quorum is a more restrictiv...
[typographical] ~199-~199: It seems that a comma is missing.
Context: ...he result to be valid. ### Yes Quorum Yes quorum is a more restrictive quorum tha...
[style] ~201-~201: You have already used this phrasing in nearby sentences. Consider replacing it to add variety to your writing.
Context: ...minimum percentage of voting power that needs to have votedYesfor the proposal to pa...
[style] ~277-~277: Three successive sentences begin with the same word. Consider rewording the sentence or use a thesaurus to find a synonym.
Context: ... quorum. *BurnProposalDepositPrevoteburns the proposal deposit if it does not ent...
[misspelling] ~285-~285: Use “an” instead of ‘a’ if the following word starts with a vowel sound, e.g. ‘an article’, ‘an hour’.
Context: ...der they were submitted. Execution has a upper limit on how much gas can be cons...
[uncategorized] ~304-~304: When specifying a month and year, the comma is unnecessary.
Context: ...validators do? * Terra crash of May, 2022, saw validators choose to run a new bin...
[formatting] ~304-~304: If the ‘because’ clause is essential to the meaning, do not use a comma before the clause.
Context: ...that had not been approved by governance, because the governance token had been inflated ...
[style] ~310-~310: Consider shortening this phrase to strengthen your wording.
Context: ...by governance. Communities whishing to make amendments to their original constitution should use ...
[style] ~320-~320: Consider using “arise” to strengthen your wording.
Context: ...nesis, so that when difficult questions come up, the constutituon can provide guidance ...
[typographical] ~339-~339: The word “however” is an adverb that can’t be used like a conjunction, and therefore needs to be separated from the sentence.
Context: ...ta` field allows custom use for networks, however, it is expected that the field contains ...
[grammar] ~357-~357: The verb form ‘are’ does not appear to fit in this context.
Context: ...keeper as a config. The default maximum length are: for the title 255 characters, for the ...
[misspelling] ~357-~357: Make sure that ‘the one of’ is correct and that ‘one’ is a pronoun. Possibly, the ‘the’ is unnecessary or ‘of’ is better expressed with a preposition such as ‘about’ or ‘in’.
Context: ... for summary 10200 characters (40 times the one of the title). #### Writing a module that...
[style] ~362-~362: As an alternative to the over-used intensifier ‘very’, consider replacing this phrase.
Context: ...as changing various parameters. This is very simple to do. First, write out your message ty...
[uncategorized] ~378-~378: Use a comma before “and” if it connects two independent clauses (unless they are closely connected and short).
Context: ...d, a new parameter set has to be created and the previous one rendered inactive. ##...
[grammar] ~445-~445: Did you mean “querying”? Or maybe you should add a pronoun? In active voice, ‘allow’ + ‘to’ takes an object, usually a pronoun.
Context: ...|'Params'toParams`. This map allows to query all x/gov params. For pseudocode p...
[uncategorized] ~450-~450: Loose punctuation mark.
Context: ...rite in stores: *load(StoreKey, Key): Retrieve item stored at keyKeyin st...
[uncategorized] ~457-~457: Loose punctuation mark.
Context: ... Store: *ProposalProcessingQueue: A queuequeue[proposalID]containing ...
[grammar] ~472-~472: Did you mean “submitting”? Or maybe you should add a pronoun? In active voice, ‘allow’ + ‘to’ takes an object, usually a pronoun.
Context: ... any messages, a legacy proposal allows to submit a set of pre-defined proposals. These p...
[uncategorized] ~509-~509: The preposition ‘to’ seems more likely in this position.
Context: ...itis reached: * PushproposalIDinProposalProcessingQueue* TransferI...
[grammar] ~521-~521: Consider using either the past participle “conformed” or the present participle “conforming” here.
Context: ...voting period * The deposited coins are conform to the accepted denom from the `MinDepo...
[style] ~539-~539: As a shorter alternative for ‘able to’, consider using “can”.
Context: ...starts. From there, bonded Atom holders are able to sendMsgVotetransactions to cast the...
[style] ~551-~551: ‘take into account’ might be wordy. Consider a shorter alternative.
Context: ...::note Gas cost for this message has to take into account the future tallying of the vote in EndB...
[typographical] ~638-~638: Two consecutive dots
Context: ... | string (address) | "cosmos1.." or empty for burn | | propos...
[uncategorized] ~643-~643: Possible missing comma found.
Context: ...nce module contains parameters that are objects unlike other modules. If only a subset ...
[grammar] ~644-~644: Make sure the noun ‘subset’ is in agreement with the verb ‘are’. Beware that some collective nouns (like ‘police’ or ‘team’) can be treated as both singular and plural.
Context: ...modules. If only a subset of parameters are desired to be changed, only they need t...
[uncategorized] ~647-~647: This expression is usually spelled with a hyphen.
Context: ...entire parameter object structure. ### Message Based Parameters In addition to the paramete...
[uncategorized] ~2465-~2465: Did you mean: “By default,”?
Context: ...t the on-chain actions they are taking. By default all metadata fields have a 255 characte...
[style] ~2465-~2465: As an alternative to the over-used intensifier ‘quite’, consider replacing this phrase.
Context: ... of proposals made by all groups may be quite large. Second, that client applications such ...
[grammar] ~2489-~2489: When ‘255-character’ is used as a modifier, it is usually spelled with a hyphen.
Context: ...Vote Location: on-chain as json within 255 character limit (mirrors [group vote](../group/RE...
Markdownlint
x/gov/README.md
36-36: Expected: 2; Actual: 4
Unordered list indentation
37-37: Expected: 2; Actual: 4
Unordered list indentation
38-38: Expected: 2; Actual: 4
Unordered list indentation
40-40: Expected: 2; Actual: 4
Unordered list indentation
41-41: Expected: 2; Actual: 4
Unordered list indentation
42-42: Expected: 2; Actual: 4
Unordered list indentation
43-43: Expected: 2; Actual: 4
Unordered list indentation
44-44: Expected: 2; Actual: 4
Unordered list indentation
45-45: Expected: 2; Actual: 4
Unordered list indentation
46-46: Expected: 2; Actual: 4
Unordered list indentation
48-48: Expected: 2; Actual: 4
Unordered list indentation
49-49: Expected: 2; Actual: 4
Unordered list indentation
50-50: Expected: 2; Actual: 4
Unordered list indentation
52-52: Expected: 2; Actual: 4
Unordered list indentation
53-53: Expected: 2; Actual: 4
Unordered list indentation
56-56: Expected: 2; Actual: 4
Unordered list indentation
57-57: Expected: 2; Actual: 4
Unordered list indentation
58-58: Expected: 2; Actual: 4
Unordered list indentation
60-60: Expected: 2; Actual: 4
Unordered list indentation
61-61: Expected: 2; Actual: 4
Unordered list indentation
301-301: Expected: 2; Actual: 4
Unordered list indentation
302-302: Expected: 2; Actual: 4
Unordered list indentation
303-303: Expected: 2; Actual: 4
Unordered list indentation
304-304: Expected: 4; Actual: 8
Unordered list indentation
306-306: Expected: 2; Actual: 4
Unordered list indentation
509-509: Expected: 2; Actual: 4
Unordered list indentation
533-533: Expected: 2; Actual: 4
Unordered list indentation
312-312: null
Emphasis used instead of a heading
27-27: null
Link fragments should be valid
62-62: null
Link fragments should be valid
Additional comments not posted (24)
x/gov/migrations/v6/store.go (1)
59-59: The addition ofProposalExecutionGastogovParamsaligns with the PR's objectives to manage gas consumption effectively. Ensure that all related components that utilizegovParamsare aware of this new field.x/gov/simulation/genesis.go (1)
197-197: The inclusion of10_000_000as a parameter in theRandomizedGenStatefunction aligns with the newProposalExecutionGasparameter, ensuring consistency in simulation environments.x/gov/keeper/abci.go (2)
80-80: The updated error handling in theEndBlockerfunction for out-of-gas errors enhances the robustness of proposal execution. This is a critical update for maintaining system stability.
Line range hint
196-227: The implementation ofExecuteWithGasLimitwithin theEndBlockerfunction is a significant enhancement. It ensures that proposal execution respects the newly introduced gas limits, aligning with the PR's objectives.x/gov/types/v1/params.go (2)
51-51: The addition ofproposalExecutionGasto theNewParamsfunction ensures that all new parameter instances will include the new gas limit setting, which is crucial for the governance module's operation.
103-103: SettingDefaultProposalExecutionGasin theDefaultParamsfunction ensures that the default settings are aligned with the new governance requirements regarding gas consumption.x/gov/proto/cosmos/gov/v1/gov.proto (1)
347-347: The addition of theproposal_execution_gasfield to theParamsmessage is well-integrated and aligns with the PR's objectives to manage gas consumption in governance proposals.x/gov/README.md (1)
281-286: The addition of theProposalExecutionGasparameter is well-explained and aligns with the PR's objectives to limit gas consumption during proposal execution.Tools
LanguageTool
[misspelling] ~285-~285: Use “an” instead of ‘a’ if the following word starts with a vowel sound, e.g. ‘an article’, ‘an hour’.
Context: ...der they were submitted. Execution has a upper limit on how much gas can be cons...api/cosmos/gov/v1/gov.pulsar.go (16)
6648-6648: Field descriptor forproposal_execution_gascorrectly added.
6675-6675: Initialization ofproposal_execution_gasfield descriptor is correct.
6869-6874: Handling ofProposalExecutionGasin conditional block is implemented correctly.
6932-6933: Case forproposal_execution_gascorrectly added to switch statement.
6992-6993: Case for resettingproposal_execution_gascorrectly added to switch statement.
7082-7084: Case for retrievingproposal_execution_gasvalue correctly added to switch statement.
7153-7154: Case for settingproposal_execution_gascorrectly added to switch statement.
7238-7239: Case for handling immutability ofproposal_execution_gascorrectly added to switch statement.
7301-7302: Case for default value ofproposal_execution_gascorrectly added to switch statement.
7459-7461: Handling for size calculation ofproposal_execution_gasimplemented correctly.
7491-7497: Handling for encodingproposal_execution_gasimplemented correctly.
8408-8426: Error handling for incorrect wire type ofproposal_execution_gasduring unmarshalling is implemented correctly.
10137-10138:ProposalExecutionGasfield correctly added to theParamsmessage.
10308-10313: Getter method forProposalExecutionGascorrectly implemented.
10572-10572: Binary encoding forProposalExecutionGasappears to be handled correctly.
10674-10749: Binary encoding forProposalExecutionGasappears to be handled correctly.
|
|
||
| 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. |
There was a problem hiding this comment.
Correct the article usage for better grammatical accuracy.
- Execution has a upper limit on how much gas can be consumed in a single block.
+ Execution has an upper limit on how much gas can be consumed in a single block.Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation.
| Execution has a upper limit on how much gas can be consumed in a single block. This limit is defined by the `ProposalExecutionGas` parameter. | |
| Execution has an upper limit on how much gas can be consumed in a single block. This limit is defined by the `ProposalExecutionGas` parameter. |
Tools
LanguageTool
[misspelling] ~285-~285: Use “an” instead of ‘a’ if the following word starts with a vowel sound, e.g. ‘an article’, ‘an hour’.
Context: ...der they were submitted. Execution has a upper limit on how much gas can be cons...
julienrbrt
changed the title
x/gov/keeper/abci.go
Outdated
| @@ -193,7 +193,11 @@ 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 { | |||
| params, err := k.Params.Get(ctx) | |||
There was a problem hiding this comment.
The params should already be there somewhere. I don't think we to get them again
| }) | ||
| 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.
| @@ -35,6 +36,11 @@ | |||
| return err | |||
| } | |||
|
|
|||
| params, err := k.Params.Get(ctx) | |||
Check warning
Code scanning / CodeQL
Panic in BeginBock or EndBlock consensus methods Warning
There was a problem hiding this comment.
Actionable comments posted: 1
Review details
Configuration used: .coderabbit.yml
Review profile: CHILL
Files selected for processing (5)
- x/accounts/go.mod (4 hunks)
- x/gov/CHANGELOG.md (1 hunks)
- x/gov/keeper/abci.go (7 hunks)
- x/gov/proto/cosmos/gov/v1/gov.proto (2 hunks)
- x/gov/types/v1/params.go (5 hunks)
Files skipped from review as they are similar to previous changes (3)
- x/gov/keeper/abci.go
- x/gov/proto/cosmos/gov/v1/gov.proto
- x/gov/types/v1/params.go
Additional context used
Path-based instructions (1)
x/gov/CHANGELOG.md (1)
Pattern
**/*.md: "Assess the documentation for misspellings, grammatical errors, missing documentation and correctness"
LanguageTool
x/gov/CHANGELOG.md
[grammar] ~42-~42: You should probably use “replicate”. (AGREEMENT_SENT_START)
Context: ...clude vote options counts. Those counts replicates the now deprecated (but not removed) ye...
[style] ~72-~72: Consider a shorter alternative to avoid wordiness. (IN_ORDER_TO_PREMIUM)
Context: ...` have been moved to the keeper package in order to support the custom tallying function. *...
Additional comments not posted (1)
x/accounts/go.mod (1)
10-10: Dependency updates are correct and match the specified versions.Also applies to: 96-96
| @@ -55,6 +55,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 exectution of proposals to a max gas limit. The limit was added to parameters and can be modified | |||
There was a problem hiding this comment.
Clarify the changelog entry for better understanding.
- * [#20348](https://github.com/cosmos/cosmos-sdk/pull/20348) Limit gov exectution of proposals to a max gas limit. The limit was added to parameters and can be modified
+ * [#20348](https://github.com/cosmos/cosmos-sdk/pull/20348) Introduce a configurable maximum gas limit for governance proposal execution, enhancing control over resource usage.Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| * [#20348](https://github.com/cosmos/cosmos-sdk/pull/20348) Limit gov exectution of proposals to a max gas limit. The limit was added to parameters and can be modified | |
| * [#20348](https://github.com/cosmos/cosmos-sdk/pull/20348) Introduce a configurable maximum gas limit for governance proposal execution, enhancing control over resource usage. |
Description
limit execution of gov proposals to infinite messages.
Author Checklist
All items are required. Please add a note to the item if the item is not applicable and
please add links to any relevant follow up issues.
I have...
!in the type prefix if API or client breaking changeCHANGELOG.mdReviewers Checklist
All items are required. Please add a note if the item is not applicable and please add
your handle next to the items reviewed if you only reviewed selected items.
I have...
Summary by CodeRabbit
New Features
Bug Fixes
Chores
cosmossdk.io/x/bankand updating versions forcosmossdk.io/logandgithub.com/hashicorp/golang-lru/v2.Refactor
ProposalExecutionGasin governance parameters.Documentation