feat(gov): add proposal types and spam votes #18532

julienrbrt · 2023-11-21T17:05:01Z

Description

ref: https://github.com/cosmos/cosmos-sdk/pull/18498/files

Prepares the groundwork for implementing optimistic proposal and multiple choice proposal

Add spam votes
Add proposal types
Add limitation to v1beta1 msg server
Add tests for spam votes
Add error when using proposal.Expedited when submitting a proposal
Add fallback from unspecified proposal to regular.
Add changelog + upgrading.md

Spam burns will be implemented in a follow-up to simplify the diff.

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...

Reviewers 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...

confirmed the correct type prefix in the PR title
confirmed ! in the type prefix if API or client breaking change
confirmed all author checklist items have been addressed
reviewed state machine logic
reviewed API design and naming
reviewed documentation is accurate
reviewed tests and test coverage
manually tested (if applicable)

coderabbitai · 2023-11-21T17:05:06Z

Walkthrough

The changes involve a significant update to a blockchain application's governance module, introducing new proposal types, deprecating the expedited field in favor of a proposal_type field, and adding a spam_count to tally results. The logic for handling proposals, voting, and tallying has been updated to accommodate these changes. Additionally, there are updates to test cases, migration scripts, and documentation to reflect the new governance features and proposal handling mechanisms.

Changes

File(s)	Change Summary
`x/gov/...`, `tests/integration/gov/...`, `x/gov/README.md`	Updated governance module logic, tests, and documentation to handle new proposal types and deprecate the `expedited` field.
`proto/cosmos/gov/v1/...`	Introduced new `ProposalType` enum, updated `VoteOption` enum, and added `spam_count` to `TallyResult`.
`x/staking`, `x/bank`, `types`	Removed or modified functions and parameters in various modules, including changes to `BurnCoins` and context creation.
`testutil/testdata/testpb/...`	Cleaned up import blocks, reordering and removing redundant imports.
`api/cosmos/nft/v1beta1/query.pulsar.go`	Added comments to RPC method type structs without changing functionality.
`x/gov/keeper/...`, `x/gov/client/...`, `x/gov/migrations/...`	Adjusted keeper, client, and migration logic for new proposal types and spam vote handling.
`x/gov/types/v1/vote.go`	Renamed and added new voting options, updated functions for backward compatibility.

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 X ?

Tips

Chat with CodeRabbit Bot (`@coderabbitai`)

If you reply to a review comment from CodeRabbit, the bot will automatically respond.
To engage with CodeRabbit bot directly around the specific lines of code in the PR, mention @coderabbitai in your review comment
Note: Review comments are made on code diffs or files, not on the PR overview.
Add @coderabbitai ignore anywhere in the PR description to prevent this PR from being reviewed.

CodeRabbit Commands (invoked as PR comments)

@coderabbitai pause to pause the reviews on a PR.
@coderabbitai resume to resume the paused reviews.
@coderabbitai review to trigger a review. This is useful when automatic reviews are disabled for the repository.
@coderabbitai resolve resolve all the CodeRabbit review comments.
@coderabbitai help to get help.

Note: For conversation with the bot, please use the review comments on code diffs or files.

CodeRabbit Configration File (`.coderabbit.yaml`)

You can programmatically configure CodeRabbit by adding a .coderabbit.yaml file to the root of your repository.
The JSON schema for the configuration file is available here.
If your editor has YAML language server enabled, you can add the path at the top of this file to enable auto-completion and validation: # yaml-language-server: $schema=https://coderabbit.ai/integrations/coderabbit-overrides.v2.json

julienrbrt · 2023-11-21T17:05:41Z

proto/cosmos/gov/v1/gov.proto

@@ -134,13 +157,15 @@ enum ProposalStatus {
 // TallyResult defines a standard tally for a governance proposal.
 message TallyResult {


todo investigate how to be minimally breaking for client if possible.

coderabbitai

Review Status

Actionable comments generated: 29

Configuration used: CodeRabbit UI

Commits

Files that changed from the base of the PR and between 875a71c and 9013310.

Files ignored due to filter (10)

api/cosmos/nft/v1beta1/query_grpc.pb.go
testutil/testdata/query.pb.go
testutil/testdata/testdata.pb.go
testutil/testdata/testpb/query_grpc.pb.go
testutil/testdata/testpb/tx_grpc.pb.go
testutil/testdata/tx.pb.go
testutil/testdata/unknonwnproto.pb.go
x/gov/types/v1/gov.pb.go
x/gov/types/v1/tx.pb.go
x/nft/query.pb.go

Files selected for processing (45)

CHANGELOG.md (1 hunks)
api/cosmos/gov/v1/gov.pulsar.go (37 hunks)
api/cosmos/gov/v1/tx.pulsar.go (17 hunks)
api/cosmos/nft/v1beta1/query.pulsar.go (10 hunks)
proto/cosmos/gov/v1/gov.proto (3 hunks)
proto/cosmos/gov/v1/tx.proto (1 hunks)
tests/integration/gov/genesis_test.go (1 hunks)
tests/integration/gov/keeper/grpc_query_test.go (2 hunks)
tests/integration/gov/keeper/tally_test.go (16 hunks)
testutil/testdata/testpb/query.pulsar.go (1 hunks)
testutil/testdata/testpb/testdata.pulsar.go (1 hunks)
testutil/testdata/testpb/tx.pulsar.go (1 hunks)
testutil/testdata/testpb/unknonwnproto.pulsar.go (1 hunks)
x/gov/CHANGELOG.md (1 hunks)
x/gov/abci.go (1 hunks)
x/gov/abci_test.go (16 hunks)
x/gov/client/cli/tx.go (2 hunks)
x/gov/client/cli/util.go (5 hunks)
x/gov/client/cli/util_test.go (2 hunks)
x/gov/client/utils/utils.go (2 hunks)
x/gov/client/utils/utils_test.go (1 hunks)
x/gov/keeper/deposit_test.go (5 hunks)
x/gov/keeper/grpc_query_test.go (18 hunks)
x/gov/keeper/hooks_test.go (2 hunks)
x/gov/keeper/keeper_test.go (2 hunks)
x/gov/keeper/migrations.go (2 hunks)
x/gov/keeper/msg_server.go (2 hunks)
x/gov/keeper/msg_server_test.go (47 hunks)
x/gov/keeper/proposal.go (2 hunks)
x/gov/keeper/proposal_test.go (7 hunks)
x/gov/keeper/tally.go (2 hunks)
x/gov/keeper/tally_test.go (22 hunks)
x/gov/keeper/vote_test.go (1 hunks)
x/gov/migrations/v3/convert_test.go (4 hunks)
x/gov/migrations/v6/store.go (1 hunks)
x/gov/migrations/v6/store_test.go (1 hunks)
x/gov/module.go (2 hunks)
x/gov/simulation/operations.go (1 hunks)
x/gov/simulation/operations_test.go (4 hunks)
x/gov/types/v1/msgs.go (1 hunks)
x/gov/types/v1/msgs_test.go (1 hunks)
x/gov/types/v1/proposal.go (2 hunks)
x/gov/types/v1/proposals_test.go (2 hunks)
x/gov/types/v1/tally.go (2 hunks)
x/gov/types/v1/vote.go (3 hunks)

Files not summarized due to errors (2)

api/cosmos/gov/v1/gov.pulsar.go: Error: Message exceeds token limit
api/cosmos/gov/v1/tx.pulsar.go: Error: Message exceeds token limit

Files skipped from review due to trivial changes (7)

api/cosmos/nft/v1beta1/query.pulsar.go
tests/integration/gov/keeper/tally_test.go
testutil/testdata/testpb/query.pulsar.go
testutil/testdata/testpb/testdata.pulsar.go
testutil/testdata/testpb/tx.pulsar.go
x/gov/keeper/msg_server_test.go
x/gov/migrations/v6/store_test.go

Additional comments: 114

x/gov/module.go (2)

38-38: The consensus version has been correctly updated to reflect the new version of the governance module. This change is necessary for the migration function Migrate5to6 to work properly.

292-294: The addition of the Migrate5to6 function is crucial for handling the transition from version 5 to 6 of the governance module. Ensure that the migration logic within Migrate5to6 correctly handles all new fields and deprecated fields, such as the transition from expedited to proposalType.

x/gov/keeper/migrations.go (2)

2-8:
The import section correctly includes the new v6 package for handling migrations from version 5 to 6.

38-45:
The new migration function Migrate5to6 is correctly implemented, calling the MigrateStore function from the v6 package. Ensure that the MigrateStore function is thoroughly tested to prevent data loss or corruption during the migration process.

testutil/testdata/testpb/unknonwnproto.pulsar.go (1)

5-18:
The import of cosmossdk.io/api/cosmos/tx/v1beta1 is new and should be verified for its usage within the file. The removal of io, math, reflect, sort, and sync suggests that their usage has been refactored or removed from the codebase. Verify that the removal of these imports does not affect any functionality and that the new import is indeed used in the code.

tests/integration/gov/keeper/grpc_query_test.go (2)

60-66: The change correctly replaces the deprecated expedited boolean with the new ProposalType enumeration. This aligns with the pull request summary indicating that the expedited field is deprecated in favor of proposalType. Ensure that all other parts of the codebase that interact with SubmitProposal are updated accordingly.

183-189: Similar to the previous hunk, the expedited boolean is replaced with ProposalType. This is consistent with the changes described in the pull request summary. Verify that the SubmitProposal function's signature has been updated everywhere it is called.

x/gov/keeper/vote_test.go (1)

23-27: The SubmitProposal function call has been updated to include the v1.ProposalType_PROPOSAL_TYPE_STANDARD argument. Ensure that all calls to SubmitProposal across the codebase are updated to include the new proposalType parameter.

tests/integration/gov/genesis_test.go (1)

80-86: The SubmitProposal function calls have been updated to include the new v1.ProposalType_PROPOSAL_TYPE_STANDARD argument. Ensure that all other parts of the codebase that call SubmitProposal are updated accordingly to pass the correct proposal type.

x/gov/keeper/keeper_test.go (2)

89-105: The test TestIncrementProposalNumber has been updated to use the new ProposalType enum values instead of boolean flags. This aligns with the changes in the governance module to support different proposal types. Ensure that all other relevant parts of the codebase are updated to use the new enum values.

116-122: The test TestProposalQueues has been updated to use the new ProposalType enum values. It's important to verify that the InactiveProposalsQueue and ActiveProposalsQueue logic correctly handles the new proposal types and that the migration of existing proposals to the new version is handled properly.

x/gov/types/v1/msgs.go (1)

18-32: The NewMsgSubmitProposal function has been updated to replace the expedited parameter with proposalType. Ensure that all invocations of this function are updated to use the new parameter. Additionally, verify that the ProposalType enumeration is properly defined and used throughout the codebase.

proto/cosmos/gov/v1/tx.proto (1)

81-93: The expedited field is deprecated and replaced by the proposal_type field. Ensure that all references to the expedited field in the codebase are updated to use proposal_type. Additionally, verify that the default behavior when proposal_type is not set aligns with the intended logic, as it defaults to PROPOSAL_TYPE_STANDARD.

CHANGELOG.md (1)

161-166:
The updates to the CHANGELOG.md accurately reflect the significant changes made to the Cosmos SDK codebase, including the removal and modification of functions and the shift in usage of certain types.

x/gov/simulation/operations.go (1)

237-255: The simulateMsgSubmitProposal function has been updated to use the new proposalType field instead of the deprecated expedited field. This change aligns with the pull request's goal to introduce structured proposal types. However, ensure that all references to the expedited field are removed or replaced throughout the entire codebase to maintain consistency.

x/gov/types/v1/proposals_test.go (2)

37-43: The test TestNestedAnys is correctly updated to use the new ProposalType enum. The use of v1.ProposalType_PROPOSAL_TYPE_STANDARD aligns with the changes described in the summary.

58-77: The test TestProposalGetMinDepositFromParams is correctly updated to use the new ProposalType enum. The test cases are checking the minimum deposit based on the proposal type, which is consistent with the changes described in the summary.

x/gov/keeper/hooks_test.go (2)

75-80: The test case has been updated to use the new ProposalType_PROPOSAL_TYPE_STANDARD instead of a boolean flag. Ensure that all references to the old expedited flag are replaced with the new proposalType field throughout the codebase.

91-94: The test case correctly captures the proposal ID from the second submission to use in the AddDeposit call. This is a good practice as it ensures that the test is interacting with the correct proposal.

x/gov/simulation/operations_test.go (4)

215-221: The NewProposal function is being called with the ProposalType_PROPOSAL_TYPE_STANDARD enum value, which is consistent with the change summary indicating the deprecation of the expedited boolean in favor of the proposalType field. This change correctly reflects the new approach to handling proposal types in the governance module.

258-264: Similar to the previous comment, the NewProposal function is correctly updated to use the ProposalType_PROPOSAL_TYPE_STANDARD enum value. This change is in line with the new proposal type handling mechanism.

302-308: Once again, the NewProposal function is correctly updated to use the ProposalType_PROPOSAL_TYPE_STANDARD enum value. This is consistent with the changes described in the pull request summary.

344-350: The NewProposal function is consistently using the ProposalType_PROPOSAL_TYPE_STANDARD enum value across all test cases, which aligns with the new proposal type handling mechanism described in the pull request summary.

x/gov/keeper/msg_server.go (2)

84-94: The code correctly sets the proposalType based on the msg.Expedited flag for backward compatibility. However, it's important to ensure that all other parts of the system that interact with proposals are updated to handle the new proposalType field instead of the deprecated expedited field. This includes updating any logic that previously relied on the expedited flag to now use the proposalType enumeration.

325-329: The legacyMsgServer struct's SubmitProposal method correctly sets the proposalType to v1.ProposalType_PROPOSAL_TYPE_STANDARD for legacy proposals. This change aligns with the deprecation of the expedited field and the introduction of the proposalType field. Ensure that the rest of the system that interacts with legacy proposals is aware of this default setting and handles it correctly.

x/gov/types/v1/proposal.go (2)

26-37: The NewProposal function has been updated to include the proposalType parameter. Ensure that all calls to this function are updated to pass the new parameter. Also, verify that the defaulting logic to ProposalType_PROPOSAL_TYPE_STANDARD when proposalType is unspecified is correctly handled in all scenarios.

53-56: The NewProposal function sets the deprecated Expedited field based on the proposalType. While this maintains backward compatibility, ensure that there is a clear deprecation path and that consumers of this field are aware of the change and migrate to using ProposalType.

x/gov/types/v1/msgs_test.go (1)

41-74: - The test cases in TestMsgSubmitProposal_GetSignBytes have been updated to include the new proposalType field and remove the deprecated expedited field. Ensure that all tests pass with the new field and that the expected sign bytes reflect the correct JSON structure with the proposalType included.

The expSignBz strings in the test cases need to be updated to include the proposalType field in the expected JSON output.

The NewMsgSubmitProposal function call within the test cases correctly uses the new proposalType field instead of the deprecated expedited field.

The fmt.Sprintf usage in line 65 should be reviewed to ensure that it correctly formats the expected JSON string with the address placeholders.

x/gov/client/cli/util_test.go (1)

223-226: The test case checks for the correct parsing of the proposal_type field from the JSON payload. However, the test case should also include a check to ensure that the deprecated expedited field is either not present or correctly handled if it appears in the payload. This is to ensure backward compatibility and proper deprecation handling.

x/gov/keeper/proposal.go (2)

19-30:
The function signature for SubmitProposal has been updated correctly to include the proposalType parameter. Ensure that all invocations of this function are updated to pass the correct proposalType.

92-95:
The NewProposal function is correctly called with the proposalType parameter. Ensure that the NewProposal function and related methods have been updated to handle this new parameter.

x/gov/types/v1/tally.go (4)

31-40: The NewTallyResult function has been correctly updated to include the new SpamCount field. Ensure that all parts of the code that call this function are updated to pass the new spam parameter.

46-49: The NewTallyResultFromMap function has been updated to handle the new OptionSpam vote option. Verify that the results map is being correctly populated with the OptionSpam key elsewhere in the code.

55-55: The EmptyTallyResult function has been correctly updated to initialize the SpamCount as zero using the math.ZeroInt() function.

59-64: The Equals method has been updated to include the SpamCount field in the comparison. This is a necessary change to ensure that tally results are compared correctly with the new field.

x/gov/migrations/v6/store.go (3)

3-8:
The import statements are correctly organized and necessary for the migration function.

10-14:
The function comment is clear and explains the purpose of the migration.

16-27:
The migration logic correctly handles the update of ProposalType. Ensure that the Expedited field is properly deprecated elsewhere in the codebase as per the pull request summary.

x/gov/migrations/v3/convert_test.go (4)

130-136: The test case "invalid no count" is consistent with the pattern of setting an invalid value and expecting an error. The use of tallyResult.SpamCount is appropriate here as it is expected to be a valid value.

140-146: The test case "invalid abstain count" follows the same pattern as the previous test cases, setting an invalid value for AbstainCount and expecting an error. The SpamCount is correctly taken from a valid tallyResult.

150-156: The test case "invalid no with veto count" is set up correctly, with an invalid value for NoWithVetoCount and an expectation of an error. The SpamCount is correctly taken from a valid tallyResult.

157-166: The test case "invalid spam count" correctly sets an invalid value for SpamCount and expects an error. This test case is important for ensuring that the new SpamCount field is properly validated.

x/gov/client/cli/tx.go (1)

141-141: The field proposal.proposalType in the NewMsgSubmitProposal function call should be accessed with a capital 'P' if it is a public member of the struct (i.e., ProposalType). If proposalType is indeed the correct field name and is public, this is fine. However, if proposalType is private or if the field name is actually ProposalType, this will result in a compilation error.

x/gov/types/v1/vote.go (3)

11-25:
The renaming and addition of constants for voting options are consistent and well-defined.

107-127:
The backward compatibility in VoteOptionFromString is handled correctly, ensuring that old string representations of vote options are mapped to the new constants.

150-160:
The ValidVoteOption function correctly validates the new voting options, including the OptionSpam.

x/gov/keeper/proposal_test.go (5)

20-39: The TestGetSetProposal function includes a TODO comment to remove it. Ensure that this is tracked in the project's task management system to avoid leaving TODOs in the code indefinitely.

48-60: Similar to the previous comment, the TestDeleteProposal function also has a TODO comment to remove it. This should be tracked and addressed.

110-127: The TestDeleteProposalInVotingPeriod function is testing the deletion of proposals in the voting period. It's important to ensure that the test cases cover all possible states of a proposal during the voting period, including before and after votes have been cast.

158-187: The TestSubmitProposal function is testing various scenarios for submitting proposals. It's crucial to verify that all new proposal types are covered in the test cases and that the expected errors are correctly asserted. Additionally, consider adding a test case for submitting a proposal with an invalid proposal type to ensure that the system handles such cases gracefully.

192-207: In the TestCancelProposal function, there are multiple proposals being submitted for testing cancellation. It's important to ensure that the test cases cover scenarios where proposals are in different states (e.g., voting not started, voting in progress, passed, rejected) to verify that cancellation behaves as expected in each case.

x/gov/client/cli/util.go (4)

10-16:
Verify that no imports were removed that are still needed elsewhere in the code.

87-98:
Ensure that the proposalType field is used appropriately in the codebase and that its privacy level does not hinder its intended use.

111-122:
The logic for parsing the proposal type and setting the proposalType field is correct.

180-193:
The logic to set the Expedited and ProposalType fields based on the FlagExpedited flag is correct, but ensure that the expedited field is indeed deprecated and replaced by ProposalType throughout the codebase.

x/gov/keeper/tally_test.go (1)

379-400: The test case "quorum reached with spam > all other votes: prop fails/burn deposit" correctly tests the new spam vote logic. However, it's important to ensure that the SpamCount is being tested against the correct logic in the Tally function, i.e., that the proposal fails if SpamCount exceeds the sum of all other votes.

proto/cosmos/gov/v1/gov.proto (6)

13-13:
The go_package option is correctly set to ensure generated Go code is placed in the appropriate package.

16-28:
The new ProposalType enum is well-defined and provides a clear structure for different types of proposals.

30-44:
The VoteOption enum is expanded correctly to include options for multiple choices and a spam option, aligning with the PR's goal to address spam votes.

119-133:
The deprecation of the expedited field in favor of the proposal_type field is consistent with the PR's description and follows best practices for evolving APIs.

157-169:
The TallyResult message is updated to include a spam_count field, which is necessary for the new spam vote tracking functionality.

158-158:
Ensure that the migration path for clients is as smooth as possible, considering Julienrbrt's previous comment about minimizing breaking changes.

x/gov/keeper/deposit_test.go (4)

24-38: The test cases for TestDeposits have been updated to use the new proposalType field instead of the deprecated expedited field. This aligns with the changes in the governance module to support multiple proposal types.

47-61: The logic for determining the depositMultiplier based on the proposal type is correct. However, ensure that the DefaultMinExpeditedDepositTokensRatio constant is properly defined and that the value is appropriate for expedited proposals.

227-233: The TestDepositAmount function has been updated to use the proposalType parameter when submitting a proposal. This change is consistent with the new governance module design.

417-423: The TestChargeDeposit function correctly uses the proposalType parameter when submitting a proposal. This is in line with the updates to the governance module.
x/gov/client/utils/utils.go (1)
3-8: The import for v1beta1 is still present but does not seem to be used anymore as references to v1beta1 have been replaced with v1. Please verify if this import can be removed to clean up the code.
-	"cosmossdk.io/x/gov/types/v1beta1"
api/cosmos/gov/v1/tx.pulsar.go (10)

133-133: The field fd_MsgSubmitProposal_expedited is still present in the code. Since the expedited field is deprecated, ensure that it is no longer being used anywhere in the codebase.

369-370: Returning the ProposalType as an enum is correct. Ensure that all corresponding enum values are properly defined and handled in the rest of the codebase.

410-410: Casting the value.Enum() to ProposalType is correct. Ensure that the ProposalType enum is properly defined.

454-454: The error message indicates that the proposal_type field is not mutable. This is consistent with the immutability of the proposal type after submission.

587-588: The size calculation for ProposalType is correct. Ensure that the runtime.Sov function correctly calculates the size of the variable-length field.

619-622: The encoding for ProposalType is correct, following the protobuf encoding rules for varint.

960-976: The unmarshalling logic for ProposalType is correct, but ensure that the ProposalType enum is properly defined and that all possible values are handled correctly elsewhere in the code.

7090-7100: The deprecation comment for the Expedited field is clear and instructive. Ensure that all references to this field are removed or updated to use ProposalType.

7165-7178: The getter methods for Expedited and ProposalType are correctly marked as deprecated and implemented, respectively. Ensure that the usage of GetExpedited is removed from the codebase.

7974-8008: The proto file descriptor setup is correct. Ensure that all new types and fields are properly documented and that their usage is consistent across the codebase.

api/cosmos/gov/v1/gov.pulsar.go (5)

1215-1215: The expedited field descriptor is still present in the gov.pulsar.go file. Since the expedited field is deprecated, ensure that it is no longer being used in the codebase and consider removing its descriptor if it's not needed.

7452-7460: The Expedited field in the Proposal message is marked as deprecated. Ensure that all references to this field in the codebase are replaced with the new ProposalType field.

8151-8158: The expedited field is still present in the Proposal message definition. Since this field is deprecated, ensure that it is no longer being used and consider removing it from the message definition if it's not needed.

8254-8268: The expedited_voting_period and expedited_min_deposit fields are present in the Params message. Since the expedited concept is deprecated, ensure that these fields are still required and update or remove them as necessary.

8291-8293: The ProposalType enum still includes PROPOSAL_TYPE_EXPEDITED. Since the expedited proposal type is deprecated, ensure that this enum value is no longer used and consider removing it if it's not needed.

x/gov/abci_test.go (14)

30-36: The NewProposal function is being used with the deprecated expedited field. Since the expedited field is deprecated in favor of proposalType, ensure that all instances of NewProposal have been updated to use the new proposalType field.

58-64: Similar to the previous comment, verify that the NewProposal function is updated to use the proposalType field instead of the deprecated expedited field.

97-103: The NewMsgSubmitProposal function call correctly uses the new proposalType field. Ensure that all other instances in the codebase are updated accordingly.

142-148: This is another correct usage of the proposalType field in the NewMsgSubmitProposal function call. Confirm that the rest of the codebase follows this pattern.

165-171: Once again, the proposalType field is used correctly in the NewMsgSubmitProposal function call. Verify consistency across the entire codebase.

251-257: The getDepositMultiplier function is used to determine the deposit amount based on the proposal type. Ensure that this logic is consistent with the new proposal type handling and that the deposit amounts are correctly calculated for each proposal type.

261-267: The NewMsgSubmitProposal function call is correct, using the proposalType from the test case struct. Confirm that the test cases cover all possible proposal types and their expected behaviors.

283-287: The test case correctly adjusts the votingPeriod based on the proposalType. Ensure that the logic for expedited proposals is correctly implemented throughout the codebase.

300-306: The test case checks for the conversion of an expedited proposal to a regular proposal after the voting period. Verify that the conversion logic is correctly implemented and that the test case accurately reflects the expected behavior.

320-333: The test cases are set up to handle different proposalType values. Ensure that the test cases cover all new proposal types introduced in the PR and that the expected outcomes are correctly asserted.

337-343: The getDepositMultiplier function is used here as well. Verify that the logic for determining the deposit multiplier is correct and that it is consistently applied across all test cases.

354-360: The SubmitProposal function call correctly uses the proposalType from the test case struct. Confirm that the proposal submission logic is consistent with the new proposal types and that the test cases accurately test the expected outcomes.

405-411: The SubmitProposal function call correctly uses the proposalType field. Ensure that the proposal handling logic is updated to work with the new proposal types and that the test cases reflect the expected behavior.

494-500: The NewMsgSubmitProposal function call is correct, using the ProposalType_PROPOSAL_TYPE_EXPEDITED for expedited proposals. Verify that the handling of expedited proposals is consistent with the new logic introduced in the PR.

x/gov/keeper/grpc_query_test.go (18)

58-64: The SubmitProposal function call has been updated to use the new ProposalType enumeration instead of the deprecated expedited boolean flag. This change aligns with the pull request's goal to introduce structured proposal types.

139-145: The SubmitProposal function call within the TestLegacyGRPCQueryProposal function has been correctly updated to use the ProposalType enumeration. This ensures that the legacy query functionality is tested with the new proposal type structure.

155-161: The SubmitProposal function call within the TestLegacyGRPCQueryProposal function has been correctly updated to use the ProposalType enumeration with the PROPOSAL_TYPE_EXPEDITED value. This tests the expedited proposal type within the legacy query functionality.

216-222: The SubmitProposal function call within the TestGRPCQueryProposals function has been correctly updated to use the ProposalType enumeration. This ensures that the proposals query functionality is tested with the new proposal type structure.

415-421: The SubmitProposal function call within the TestLegacyGRPCQueryProposals function has been correctly updated to use the ProposalType enumeration. This ensures that the legacy proposals query functionality is tested with the new proposal type structure.

496-502: The SubmitProposal function call within the TestGRPCQueryVote function has been correctly updated to use the ProposalType enumeration. This ensures that the vote query functionality is tested with the new proposal type structure.

611-617: The SubmitProposal function call within the TestLegacyGRPCQueryVote function has been correctly updated to use the ProposalType enumeration. This ensures that the legacy vote query functionality is tested with the new proposal type structure.

718-724: The SubmitProposal function call within the TestGRPCQueryVotes function has been correctly updated to use the ProposalType enumeration. This ensures that the votes query functionality is tested with the new proposal type structure.

822-828: The SubmitProposal function call within the TestLegacyGRPCQueryVotes function has been correctly updated to use the ProposalType enumeration. This ensures that the legacy votes query functionality is tested with the new proposal type structure.

1110-1116: The SubmitProposal function call within the TestGRPCQueryDeposit function has been correctly updated to use the ProposalType enumeration. This ensures that the deposit query functionality is tested with the new proposal type structure.

1212-1218: The SubmitProposal function call within the TestLegacyGRPCQueryDeposit function has been correctly updated to use the ProposalType enumeration. This ensures that the legacy deposit query functionality is tested with the new proposal type structure.

1303-1309: The SubmitProposal function call within the TestGRPCQueryDeposits function has been correctly updated to use the ProposalType enumeration with the PROPOSAL_TYPE_EXPEDITED value. This tests the expedited proposal type within the deposits query functionality.

1400-1406: The SubmitProposal function call within the TestLegacyGRPCQueryDeposits function has been correctly updated to use the ProposalType enumeration. This ensures that the legacy deposits query functionality is tested with the new proposal type structure.

1502-1508: The TallyResult struct has been correctly updated to include the new SpamCount field. This change is part of the pull request's goal to address spam votes in governance proposals.

1519-1525: The TallyResult struct within the TestGRPCQueryTallyResult function has been correctly updated to include the new SpamCount field. This ensures that the tally result query functionality is tested with the new spam vote tracking feature.

1546-1552: The TallyResult struct within the TestGRPCQueryTallyResult function has been correctly updated to include the new SpamCount field. This ensures that the tally result query functionality is tested with the new spam vote tracking feature for proposals in the deposit period.

1573-1579: The TallyResult struct within the TestGRPCQueryTallyResult function has been correctly updated to include the new SpamCount field. This ensures that the tally result query functionality is tested with the new spam vote tracking feature for proposals in the voting period.

1645-1651: The TallyResult struct within the TestLegacyGRPCQueryTallyResult function has been correctly updated to include the new SpamCount field. This ensures that the legacy tally result query functionality is tested with the new spam vote tracking feature.

x/gov/abci.go

x/gov/types/v1/proposals_test.go

x/gov/CHANGELOG.md

x/gov/client/cli/util_test.go

x/gov/client/cli/tx.go

api/cosmos/gov/v1/tx.pulsar.go

api/cosmos/gov/v1/gov.pulsar.go

coderabbitai

Review Status

Actionable comments generated: 7

Configuration used: CodeRabbit UI

Commits

Files that changed from the base of the PR and between 9013310 and 7eed169.

Files ignored due to filter (1)

x/gov/types/v1/gov.pb.go

Files selected for processing (7)

api/cosmos/gov/v1/gov.pulsar.go (37 hunks)
proto/cosmos/gov/v1/gov.proto (3 hunks)
tests/integration/bank/app_test.go (1 hunks)
x/gov/CHANGELOG.md (1 hunks)
x/gov/README.md (8 hunks)
x/gov/migrations/v6/store.go (1 hunks)
x/gov/types/v1/vote.go (3 hunks)

Files not summarized due to errors (1)

api/cosmos/gov/v1/gov.pulsar.go: Error: Message exceeds token limit

Files skipped from review due to trivial changes (1)

x/gov/README.md

Additional comments: 13

tests/integration/bank/app_test.go (1)

386-390:
The update to use govv1.ProposalType_PROPOSAL_TYPE_STANDARD instead of a boolean value aligns with the new ProposalType enumeration. Ensure that all related test cases and code paths have been updated accordingly.

x/gov/CHANGELOG.md (1)

27-44:

The changelog entries are clear and correctly link to the associated pull request. The deprecation of the Expedited field is correctly documented, consistent with the changes made in the pull request.

x/gov/migrations/v6/store.go (1)

1-33:

The migration logic correctly updates the ProposalType based on the deprecated Expedited field. However, ensure that the Expedited field is removed from the Proposal structure after the migration to avoid confusion and maintain clean code.

x/gov/types/v1/vote.go (2)

13-25:
Verify that all parts of the codebase and any external systems interacting with these constants are updated to reflect the new definitions.

139-148:
The changes to the ValidVoteOption function correctly reflect the addition of the new vote options.

proto/cosmos/gov/v1/gov.proto (5)

16-28:
The addition of ProposalType enum is correctly implemented to support different types of proposals.

32-53:
The update to VoteOption enum to include aliases and a new VOTE_OPTION_SPAM is correctly implemented.

129-143:
The deprecation of the expedited field in favor of proposal_type aligns with the PR's intent to introduce structured proposal types.

167-179:
The addition of spam_count to TallyResult is correctly implemented to track spam votes.

168-168:
Consider providing a detailed migration guide for clients to handle the transition from expedited to proposal_type smoothly.

api/cosmos/gov/v1/gov.pulsar.go (3)

7605-7610: The GetProposalType method correctly handles the zero value for ProposalType, which aligns with the changes in the pull request summary.

8300-8309: The ProposalType enum is correctly defined with the new proposal types as described in the pull request summary.

8310-8324: The VoteOption enum includes a new VOTE_OPTION_SPAM as described in the pull request summary, which is used to track spam votes.

coderabbitai · 2023-11-22T20:23:53Z

x/gov/types/v1/vote.go

@@ -103,7 +109,7 @@ func (v WeightedVoteOptions) String() string {
 func VoteOptionFromString(str string) (VoteOption, error) {
 	option, ok := VoteOption_value[str]
 	if !ok {
-		return OptionEmpty, fmt.Errorf("'%s' is not a valid vote option, available options: yes/no/no_with_veto/abstain", str)
+		return OptionEmpty, fmt.Errorf("'%s' is not a valid vote option, available options: yes,option_one/no,option_three/no_with_veto,option_four/abstain,option_two/spam", str)


The error message should be updated to reflect the new vote options.

- return OptionEmpty, fmt.Errorf("'%s' is not a valid vote option, available options: yes,option_one/no,option_three/no_with_veto,option_four/abstain,option_two/spam", str) + return OptionEmpty, fmt.Errorf("'%s' is not a valid vote option, available options: option_one, option_two, option_three, option_four, spam", str)

Commitable 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.

Suggested change

return OptionEmpty, fmt.Errorf("'%s' is not a valid vote option, available options: yes,option_one/no,option_three/no_with_veto,option_four/abstain,option_two/spam", str)

return OptionEmpty, fmt.Errorf("'%s' is not a valid vote option, available options: option_one, option_two, option_three, option_four, spam", str)

coderabbitai · 2023-11-22T20:23:53Z

api/cosmos/gov/v1/gov.pulsar.go

@@ -1212,6 +1212,7 @@ var (
 	fd_Proposal_proposer           protoreflect.FieldDescriptor
 	fd_Proposal_expedited          protoreflect.FieldDescriptor
 	fd_Proposal_failed_reason      protoreflect.FieldDescriptor
+	fd_Proposal_proposal_type      protoreflect.FieldDescriptor


The expedited field is marked as deprecated in the pull request summary, but it is still present in the fd_Proposal_expedited field descriptor. Please ensure that the deprecation is correctly reflected in the codebase.

coderabbitai · 2023-11-22T20:23:54Z

api/cosmos/gov/v1/gov.pulsar.go

+	if x.SpamCount != "" {
+		value := protoreflect.ValueOfString(x.SpamCount)
+		if !f(fd_TallyResult_spam_count, value) {
+			return
+		}
+	}


The SpamCount field in TallyResult is being treated as a string, which is inconsistent with the summary stating that spam_count is a new field to track spam votes. This should be an integer or a numeric type to properly count votes.

- SpamCount string `protobuf:"bytes,5,opt,name=spam_count,json=spamCount,proto3" json:"spam_count,omitempty"` + SpamCount int64 `protobuf:"varint,5,opt,name=spam_count,json=spamCount,proto3" json:"spam_count,omitempty"`

Commitable 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.

Suggested change

if x.SpamCount != "" {

value := protoreflect.ValueOfString(x.SpamCount)

if !f(fd_TallyResult_spam_count, value) {

return

}

}

coderabbitai · 2023-11-22T20:23:54Z

api/cosmos/gov/v1/gov.pulsar.go

+	0x12, 0x20, 0x0a, 0x09, 0x65, 0x78, 0x70, 0x65, 0x64, 0x69, 0x74, 0x65, 0x64, 0x18, 0x0e, 0x20,
+	0x01, 0x28, 0x08, 0x42, 0x02, 0x18, 0x01, 0x52, 0x09, 0x65, 0x78, 0x70, 0x65, 0x64, 0x69, 0x74,
+	0x65, 0x64, 0x12, 0x23, 0x0a, 0x0d, 0x66, 0x61, 0x69, 0x6c, 0x65, 0x64, 0x5f, 0x72, 0x65, 0x61,
+	0x73, 0x6f, 0x6e, 0x18, 0x0f, 0x20, 0x01, 0x28, 0x09, 0x52, 0x0c, 0x66, 0x61, 0x69, 0x6c, 0x65,
+	0x64, 0x52, 0x65, 0x61, 0x73, 0x6f, 0x6e, 0x12, 0x40, 0x0a, 0x0d, 0x70, 0x72, 0x6f, 0x70, 0x6f,
+	0x73, 0x61, 0x6c, 0x5f, 0x74, 0x79, 0x70, 0x65, 0x18, 0x10, 0x20, 0x01, 0x28, 0x0e, 0x32, 0x1b,
+	0x2e, 0x63, 0x6f, 0x73, 0x6d, 0x6f, 0x73, 0x2e, 0x67, 0x6f, 0x76, 0x2e, 0x76, 0x31, 0x2e, 0x50,
+	0x72, 0x6f, 0x70, 0x6f, 0x73, 0x61, 0x6c, 0x54, 0x79, 0x70, 0x65, 0x52, 0x0c, 0x70, 0x72, 0x6f,
+	0x70, 0x6f, 0x73, 0x61, 0x6c, 0x54, 0x79, 0x70, 0x65, 0x22, 0x86, 0x02, 0x0a, 0x0b, 0x54, 0x61,


The expedited field is marked as deprecated in the pull request summary, but it is still present in the Proposal message. Ensure that the deprecation is correctly reflected in the codebase and that the field is removed if it is no longer needed.

coderabbitai · 2023-11-22T20:23:54Z

api/cosmos/gov/v1/gov.pulsar.go

+	0x72, 0x75, 0x6d, 0x12, 0x41, 0x0a, 0x1d, 0x62, 0x75, 0x72, 0x6e, 0x5f, 0x70, 0x72, 0x6f, 0x70,
+	0x6f, 0x73, 0x61, 0x6c, 0x5f, 0x64, 0x65, 0x70, 0x6f, 0x73, 0x69, 0x74, 0x5f, 0x70, 0x72, 0x65,
+	0x76, 0x6f, 0x74, 0x65, 0x18, 0x0e, 0x20, 0x01, 0x28, 0x08, 0x52, 0x1a, 0x62, 0x75, 0x72, 0x6e,
+	0x50, 0x72, 0x6f, 0x70, 0x6f, 0x73, 0x61, 0x6c, 0x44, 0x65, 0x70, 0x6f, 0x73, 0x69, 0x74, 0x50,


The burn_proposal_deposit_pre_vote field seems to be a boolean flag, but it is not clear from the field name what its purpose is. Consider adding a comment to explain its use or renaming it for clarity.

coderabbitai · 2023-11-22T20:23:54Z

api/cosmos/gov/v1/gov.pulsar.go

+	// Deprecated: Use ProposalType instead.
+	//
+	// Deprecated: Do not use.
 	Expedited bool `protobuf:"varint,14,opt,name=expedited,proto3" json:"expedited,omitempty"`


The Expedited field is marked as deprecated, but it is still present in the Proposal message. Ensure that the deprecation is correctly reflected in the codebase and that the field is removed if it is no longer needed.

coderabbitai · 2023-11-22T20:23:54Z

api/cosmos/gov/v1/gov.pulsar.go

+func (x *TallyResult) GetSpamCount() string {
+	if x != nil {
+		return x.SpamCount
+	}
+	return ""
+}


The GetSpamCount method is returning a string, which is inconsistent with the summary stating that spam_count is a new field to track spam votes. This should return an integer or a numeric type.

odeke-em

Nice work and thank you @julienrbrt, just some drive-by comments.

proto/cosmos/gov/v1/gov.proto

x/gov/client/cli/util.go

x/gov/migrations/v6/store.go

coderabbitai

Review Status

Actionable comments generated: 3

Configuration used: CodeRabbit UI

Commits

Files that changed from the base of the PR and between 7eed169 and f0464f5.

Files ignored due to filter (1)

proto/buf.yaml

Files selected for processing (3)

x/gov/client/cli/util.go (5 hunks)
x/gov/client/utils/utils.go (2 hunks)
x/gov/migrations/v6/store.go (1 hunks)

Additional comments: 5

x/gov/client/cli/util.go (4)

4-17:
The addition of the "strings" and "govv1" imports aligns with the usage in the updated functions. Ensure that the "v1beta1" import is still needed after the refactoring.

50-59:
The logic to handle the case where the proposal file is not provided and to parse the proposal type from flags is correctly implemented.

88-99:
The new proposal struct is well-defined and includes the necessary fields to support the new proposal types.

112-123:
The logic to parse the proposal type and messages from the proposal JSON is correctly implemented.

x/gov/migrations/v6/store.go (1)

10-29:

The migration logic correctly updates the ProposalType based on the deprecated Expedited field, aligning with the changes described in the pull request summary. Ensure that all relevant documentation and client-side code are updated to reflect these changes.

x/gov/client/utils/utils.go

x/gov/client/cli/util.go

x/gov/types/v1/msgs.go

coderabbitai

Review Status

Actionable comments generated: 0

Configuration used: CodeRabbit UI

Commits

Files that changed from the base of the PR and between b000ac0 and 96cb2d9.

Files selected for processing (1)

CHANGELOG.md (1 hunks)

Additional comments: 1

CHANGELOG.md (1)

155-160: The changes documented in the CHANGELOG.md align with the PR objectives and the AI-generated summaries, confirming the removal of WrapServiceResult, the replacement of "cosmossdk.io/x/staking/types".Infraction_* with "cosmossdk.io/api/cosmos/staking/v1beta1".Infraction_, the change in NewContext to no longer accept cmtproto.Header{}, and the modification in BurnCoins to take an address instead of a module name. These updates are consistent with the intended enhancements to the governance module of the Cosmos SDK.

julienrbrt · 2023-12-06T15:37:12Z

@coderabbitai pause

coderabbitai

Review Status

Actionable comments generated: 2

Configuration used: CodeRabbit UI

Commits

Files that changed from the base of the PR and between 96cb2d9 and 2d105ee.

Files selected for processing (7)

x/gov/abci.go (5 hunks)
x/gov/abci_test.go (17 hunks)
x/gov/keeper/deposit.go (2 hunks)
x/gov/keeper/msg_server.go (2 hunks)
x/gov/migrations/v3/convert_test.go (4 hunks)
x/gov/types/v1/msgs_test.go (1 hunks)
x/gov/types/v1/proposals_test.go (2 hunks)

Additional comments: 32

x/gov/abci.go (5)

84-90: The logging changes reflect the updated proposal structure and are consistent with the PR objectives.

131-137: The logic to handle deposits for expedited proposals is correctly updated to use the new ProposalType field.

199-211: The handling of failed expedited proposals, including the conversion to regular proposals and the extension of the voting period, aligns with the PR objectives.

247-253: The logging of the proposal tally results is updated to include the new ProposalType field, which is consistent with the PR objectives.

318-324: The logging for a proposal that failed to decode is updated to include the new ProposalType field, which is consistent with the PR objectives.

x/gov/abci_test.go (16)

30-36: The test function TestUnregisteredProposal_InactiveProposalFails has been updated to use the v1.ProposalType_PROPOSAL_TYPE_STANDARD enum value instead of a boolean flag for the proposal type. This change aligns with the PR objectives to introduce new proposal types and deprecate the expedited field.

58-64: The test function TestUnregisteredProposal_ActiveProposalFails has also been updated to use the v1.ProposalType_PROPOSAL_TYPE_STANDARD enum value. The status of the proposal is explicitly set to v1.StatusVotingPeriod, and the voting end time is set, which is necessary for the test logic.

97-103: The test function TestTickExpiredDepositPeriod has been updated to include the v1.ProposalType_PROPOSAL_TYPE_STANDARD enum value in the NewMsgSubmitProposal call. This change is consistent with the updates to the governance module.

142-148: The test function TestTickMultipleExpiredDepositPeriod has been updated similarly to TestTickExpiredDepositPeriod, using the v1.ProposalType_PROPOSAL_TYPE_STANDARD enum value in the NewMsgSubmitProposal call.

165-171: The test function TestTickMultipleExpiredDepositPeriod has another instance of NewMsgSubmitProposal being updated with the v1.ProposalType_PROPOSAL_TYPE_STANDARD enum value.

205-211: The test function TestTickPassedDepositPeriod has been updated to include the v1.ProposalType_PROPOSAL_TYPE_STANDARD enum value in the NewMsgSubmitProposal call.

251-257: The setup for the TestTickPassedVotingPeriod test cases includes a call to getDepositMultiplier to adjust the deposit amount based on the proposal type. This is a good practice as it reflects the different requirements for different proposal types.

261-267: The TestTickPassedVotingPeriod test function correctly uses the proposalType from the test case struct when creating a new proposal message.

282-287: The TestTickPassedVotingPeriod test function includes logic to adjust the voting period based on the proposal type, which is consistent with the governance module's new functionality.

300-306: The TestTickPassedVotingPeriod test function includes a check to ensure that expedited proposals are converted to regular proposals after the expedited voting period ends.

311-315: The TestTickPassedVotingPeriod test function asserts that after the expedited voting period, the proposal type should not be expedited and the voting end time should be set correctly. This is a critical check to ensure the correct behavior of expedited proposals.

337-343: The TestProposalPassedEndblocker test function setup includes a call to getDepositMultiplier to adjust the deposit amount based on the proposal type, similar to the TestTickPassedVotingPeriod test function.

354-360: The TestProposalPassedEndblocker test function correctly submits a proposal with the proposalType from the test case struct.

405-411: The TestEndBlockerProposalHandlerFailed test function submits a proposal with the v1.ProposalType_PROPOSAL_TYPE_STANDARD enum value, which is consistent with the changes to the governance module.

494-500: The TestExpeditedProposal_PassAndConversionToRegular test function submits an expedited proposal using the v1.ProposalType_PROPOSAL_TYPE_EXPEDITED enum value, which is consistent with the new proposal types introduced in the governance module.

559-565: The TestExpeditedProposal_PassAndConversionToRegular test function includes checks to ensure that an expedited proposal is converted to a regular proposal after the expedited voting period, and that the voting end time is set correctly.

x/gov/keeper/deposit.go (4)

281-287: The validation logic for the initial deposit checks if the deposit is valid and not negative, which is good practice for ensuring data integrity.

298-303: Verify that the ProposalType_PROPOSAL_TYPE_EXPEDITED case is still required in the switch statement, as the PR objective mentions deprecating the expedited field.

Verification result:

The ProposalType_PROPOSAL_TYPE_EXPEDITED case within the switch statement in deposit.go is still valid and required. The usage of ProposalType_PROPOSAL_TYPE_EXPEDITED is found in multiple places within the codebase, including tests, message server logic, and migration scripts. This indicates that while the expedited field may be deprecated, the expedited proposal type is still recognized and handled in various parts of the governance module.

295-304: > Note: This review was outside the patches, so it was mapped to the patch with the greatest overlap. Original lines [304-309]

The calculation of minDepositCoins using minInitialDepositRatio is correctly placed after the switch statement to apply to all cases.

281-287: > Note: This review was outside the patches, and no patch overlapping with it was found. Original lines [310-313]

The final validation check to ensure the initialDeposit meets the minimum required minDepositCoins is correctly implemented.

x/gov/keeper/msg_server.go (2)

76-95: The changes in the SubmitProposal function correctly handle the deprecation of the Expedited field by introducing a proposalType variable and using it in subsequent function calls. This aligns with the PR objectives and the generated summaries.

326-332: The changes in the SubmitProposal function of the legacyMsgServer type correctly set legacy proposals to only be of standard type by using v1.ProposalType_PROPOSAL_TYPE_STANDARD. This aligns with the PR objectives and the generated summaries.

x/gov/migrations/v3/convert_test.go (1)

130-136: > Note: This review was outside the patches, so it was mapped to the patch with the greatest overlap. Original lines [130-156]

The test cases for "invalid no count", "invalid abstain count", and "invalid no with veto count" are correctly setting invalid values for their respective fields and expecting an error, which is consistent with the test case's purpose.

x/gov/types/v1/msgs_test.go (1)

41-74: The changes in the test function TestMsgSubmitProposal_GetSignBytes correctly reflect the updates to the MsgSubmitProposal structure, replacing the expedited boolean field with the proposalType field of type v1.ProposalType. The test cases have been updated accordingly, and the expected JSON strings in the test cases have been modified to include the proposal_type attribute. This aligns with the PR objectives and the AI-generated summaries.
x/gov/types/v1/proposals_test.go (3)
47-57: The Expedited field is still being referenced and used in tests, which is inconsistent with the PR objectives and the AI-generated summaries that mention the deprecation of the expedited field. This field should be removed or updated to align with the new proposal_type field.
- require.Equal(t, startExpedited, proposal.Expedited)
- require.Equal(t, !startExpedited, proposal.Expedited)
40-40: The updated NewProposal function signature correctly takes a ProposalType argument, aligning with the PR objectives and the AI-generated summaries.

40-40: The ProposalType enum is being used correctly in the tests to create proposals of different types, which is consistent with the PR objectives and the AI-generated summaries.

Also applies to: 54-54

x/gov/migrations/v3/convert_test.go

coderabbitai

Review Status

Actionable comments generated: 2

Configuration used: CodeRabbit UI

Commits

Files that changed from the base of the PR and between 715d5f1 and ebd6ab8.

Files selected for processing (4)

tests/integration/gov/keeper/grpc_query_test.go (4 hunks)
x/gov/abci_test.go (19 hunks)
x/gov/keeper/deposit_test.go (6 hunks)
x/gov/keeper/export_test.go (1 hunks)

Files skipped from review as they are similar to previous changes (1)

x/gov/abci_test.go

Additional comments: 4

tests/integration/gov/keeper/grpc_query_test.go (3)

15-17: The summary indicates that TestGRPCQueryTally has been removed, but the code shows TestLegacyGRPCQueryTally is present. Please clarify if the summary is outdated or if the function was renamed rather than removed.

33-34: The expErrMsg field in the test case structure is not used and could be removed to simplify the test code.

75-77: Consider adding more test cases to ensure comprehensive coverage of the new proposal types and spam votes functionality introduced in the PR.

x/gov/keeper/export_test.go (1)

11-17: The update to the ValidateInitialDeposit function signature to accept a proposalType parameter is consistent with the PR's objective to handle different proposal types. Ensure that all calls to this function are updated to pass the correct proposalType.

x/gov/keeper/deposit_test.go

x/gov/abci_test.go

* feat: secp256k1 public key constant time (cosmos#18026) Signed-off-by: bizk <[email protected]> * chore: Fixed changelog duplicated items (cosmos#18628) * adr: Un-Ordered Transaction Inclusion (cosmos#18553) Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com> Co-authored-by: Aleksandr Bezobchuk <[email protected]> * docs: lint ADR-070 (cosmos#18634) * fix(baseapp)!: postHandler should run regardless of result (cosmos#18627) * docs: fix typos in adr-007-specialization-groups.md (cosmos#18635) * chore: alphabetize labels (cosmos#18640) * docs(x/circuit): add note on ante handler (cosmos#18637) Co-authored-by: Aleksandr Bezobchuk <[email protected]> * fix: telemetry metric label variable (cosmos#18643) * chore: typos fix (cosmos#18642) * refactor(store/v2): updates from integration (cosmos#18633) * build(deps): Bump actions/setup-go from 4 to 5 (cosmos#18647) Signed-off-by: dependabot[bot] <[email protected]> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> Co-authored-by: github-actions <41898282+github-actions[bot]@users.noreply.github.com> * feat(store/v2): snapshot manager (cosmos#18458) * chore(client/v2): fix typos in the README.md (cosmos#18657) * fix(baseapp): protocompat.go gogoproto.Merge does not work with custom types (cosmos#18654) Co-authored-by: unknown unknown <unknown@unknown> * chore: fix several minor typos (cosmos#18660) * chore(tools/confix/cmd): fix typo in view.go (cosmos#18659) * refactor(x/staking): check duplicate addresses in StakeAuthorization's params (cosmos#18655) * feat(accounts): use gogoproto API instead of protov2. (cosmos#18653) Co-authored-by: unknown unknown <unknown@unknown> Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com> * fix(store/commitment/iavl): honor tree.Remove error firstly (cosmos#18651) * build(deps): Bump actions/stale from 8 to 9 (cosmos#18656) Signed-off-by: dependabot[bot] <[email protected]> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> * chore(docs): fix typos & wording in docs (cosmos#18667) * chore: fix several typos. (cosmos#18666) * feat(telemetry): enable `statsd` and `dogstatsd` telemetry sinks (cosmos#18646) Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com> Co-authored-by: Aleksandr Bezobchuk <[email protected]> Co-authored-by: marbar3778 <[email protected]> Co-authored-by: Marko <[email protected]> * feat(store/v2): add SetInitialVersion in SC (cosmos#18665) * feat(client/keys): support display discreetly for `keys add` (cosmos#18663) Co-authored-by: Julien Robert <[email protected]> * ci: add misspell action (cosmos#18671) * chore: typos fix by misspell-fixer (cosmos#18683) Co-authored-by: github-merge-queue <[email protected]> Co-authored-by: Julien Robert <[email protected]> * chore: add v0.50.2 changelog to main (cosmos#18682) * build(deps): Bump github.com/jhump/protoreflect from 1.15.3 to 1.15.4 in /tests (cosmos#18678) Signed-off-by: dependabot[bot] <[email protected]> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> * refactor(bank): remove .String() calls (cosmos#18175) Co-authored-by: Facundo <[email protected]> * ci: use codespell instead of misspell-fixer (cosmos#18686) Co-authored-by: Marko <[email protected]> * feat(gov): add proposal types and spam votes (cosmos#18532) * feat(accounts): use account number as state prefix for account state (cosmos#18664) Co-authored-by: unknown unknown <unknown@unknown> * chore: typos fixes by cosmos-sdk bot (cosmos#18689) Co-authored-by: github-merge-queue <[email protected]> Co-authored-by: Julien Robert <[email protected]> Co-authored-by: marbar3778 <[email protected]> * feat(client/keys): support display discreetly for keys mnemonic (cosmos#18688) * refactor: remove panic usage in keeper methods (cosmos#18636) * ci: rename pr name in misspell job (cosmos#18693) Co-authored-by: Marko <[email protected]> * build(deps): Bump github.com/pelletier/go-toml/v2 from 2.1.0 to 2.1.1 in /tools/confix (cosmos#18702) Signed-off-by: dependabot[bot] <[email protected]> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> Co-authored-by: github-actions <41898282+github-actions[bot]@users.noreply.github.com> * feat(client/keys): support display discreetly for keys export (cosmos#18684) * feat(x/gov): better gov genesis validation (cosmos#18707) --------- Signed-off-by: bizk <[email protected]> Signed-off-by: dependabot[bot] <[email protected]> Co-authored-by: Carlos Santiago Yanzon <[email protected]> Co-authored-by: yihuang <[email protected]> Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com> Co-authored-by: Aleksandr Bezobchuk <[email protected]> Co-authored-by: Facundo Medica <[email protected]> Co-authored-by: Akaonetwo <[email protected]> Co-authored-by: Marko <[email protected]> Co-authored-by: Julien Robert <[email protected]> Co-authored-by: dreamweaverxyz <[email protected]> Co-authored-by: Pioua <[email protected]> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> Co-authored-by: github-actions <41898282+github-actions[bot]@users.noreply.github.com> Co-authored-by: cool-developer <[email protected]> Co-authored-by: leonarddt05 <[email protected]> Co-authored-by: testinginprod <[email protected]> Co-authored-by: unknown unknown <unknown@unknown> Co-authored-by: Sukey <[email protected]> Co-authored-by: axie <[email protected]> Co-authored-by: Luke Ma <[email protected]> Co-authored-by: Emmanuel T Odeke <[email protected]> Co-authored-by: 0xn4de <[email protected]> Co-authored-by: hattizai <[email protected]> Co-authored-by: Devon Bear <[email protected]> Co-authored-by: Marko <[email protected]> Co-authored-by: Halimao <[email protected]> Co-authored-by: Cosmos SDK <[email protected]> Co-authored-by: github-merge-queue <[email protected]> Co-authored-by: Facundo <[email protected]> Co-authored-by: Likhita Polavarapu <[email protected]>

julienrbrt added 2 commits November 21, 2023 18:01

feat(gov): add proposal types and spam votes

62ef693

updates

2a536e8

github-actions bot added C:CLI C:x/gov C:x/nft labels Nov 21, 2023

julienrbrt commented Nov 21, 2023

View reviewed changes

julienrbrt added 2 commits November 22, 2023 00:53

wip

e8da7ac

fix tests

fb281bf

github-actions bot added the C:Simulations label Nov 22, 2023

julienrbrt added 4 commits November 22, 2023 17:29

updates

a568dc9

updates

50f1015

validate previous options

c9dc8c8

Merge branch 'main' into julien/gov-spam-proposal-type

9013310

julienrbrt marked this pull request as ready for review November 22, 2023 17:39

julienrbrt requested a review from a team as a code owner November 22, 2023 17:39

coderabbitai bot reviewed Nov 22, 2023

View reviewed changes

julienrbrt marked this pull request as draft November 22, 2023 19:47

updates

7eed169

julienrbrt marked this pull request as ready for review November 22, 2023 20:22

coderabbitai bot reviewed Nov 22, 2023

View reviewed changes

julienrbrt requested review from alexanderbez, facundomedica and tac0turtle November 22, 2023 20:26

proto lint

411378e

julienrbrt added a commit that referenced this pull request Nov 22, 2023

match with #18532 implementation

e3452b9

odeke-em reviewed Nov 24, 2023

View reviewed changes

proto/cosmos/gov/v1/gov.proto Show resolved Hide resolved

proto/cosmos/gov/v1/gov.proto Show resolved Hide resolved

x/gov/client/cli/util.go Outdated Show resolved Hide resolved

x/gov/migrations/v6/store.go Outdated Show resolved Hide resolved

nits

f0464f5

coderabbitai bot reviewed Nov 24, 2023

View reviewed changes

x/gov/client/utils/utils.go Outdated Show resolved Hide resolved

x/gov/client/utils/utils.go Show resolved Hide resolved

x/gov/client/cli/util.go Show resolved Hide resolved

remove duplicate tests

2e62db1

Merge branch 'main' into julien/gov-spam-proposal-type

4aa5df7

julienrbrt mentioned this pull request Dec 4, 2023

feat(x/gov): optimistic proposals #18620

Merged

12 tasks

facundomedica reviewed Dec 4, 2023

View reviewed changes

x/gov/types/v1/msgs.go Show resolved Hide resolved

Merge branch 'main' into julien/gov-spam-proposal-type

96cb2d9

coderabbitai bot reviewed Dec 6, 2023

View reviewed changes

updates

2d105ee

Merge branch 'main' into julien/gov-spam-proposal-type

715d5f1

coderabbitai bot reviewed Dec 6, 2023

View reviewed changes

x/gov/migrations/v3/convert_test.go Show resolved Hide resolved

updates

ebd6ab8

coderabbitai bot reviewed Dec 6, 2023

View reviewed changes

x/gov/keeper/deposit_test.go Show resolved Hide resolved

x/gov/keeper/deposit_test.go Outdated Show resolved Hide resolved

typo

4d28273

julienrbrt enabled auto-merge December 6, 2023 20:47

julienrbrt requested review from odeke-em, facundomedica and tac0turtle December 6, 2023 20:47

julienrbrt disabled auto-merge December 6, 2023 20:47

tac0turtle approved these changes Dec 11, 2023

View reviewed changes

julienrbrt added 3 commits December 11, 2023 11:57

Merge branch 'main' into julien/gov-spam-proposal-type

ba7649f

Merge branch 'main' into julien/gov-spam-proposal-type

59654b6

updates

f08d2f5

coderabbitai bot reviewed Dec 11, 2023

View reviewed changes

x/gov/abci_test.go Show resolved Hide resolved

julienrbrt added this pull request to the merge queue Dec 11, 2023

Merged via the queue into main with commit 8b894f7 Dec 11, 2023
60 of 62 checks passed

julienrbrt deleted the julien/gov-spam-proposal-type branch December 11, 2023 11:42

marcello33 mentioned this pull request Jan 22, 2024

Upstream fetch (#2) #19163

Closed

12 tasks

This was referenced Apr 18, 2024

feat(x/gov): add MaxVoteOptionsLen #20087

Merged

chore: sync main changelogs with v0.47.11, v0.50.6, modules and x/tx #20125

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat(gov): add proposal types and spam votes #18532

feat(gov): add proposal types and spam votes #18532

julienrbrt commented Nov 21, 2023 •

edited by coderabbitai bot

Loading

coderabbitai bot commented Nov 21, 2023 •

edited

Loading

Chat with CodeRabbit Bot (`@coderabbitai`)

CodeRabbit Commands (invoked as PR comments)

CodeRabbit Configration File (`.coderabbit.yaml`)

julienrbrt Nov 21, 2023

coderabbitai bot left a comment

coderabbitai bot left a comment

coderabbitai bot Nov 22, 2023

coderabbitai bot Nov 22, 2023

coderabbitai bot Nov 22, 2023

coderabbitai bot Nov 22, 2023

coderabbitai bot Nov 22, 2023

coderabbitai bot Nov 22, 2023

coderabbitai bot Nov 22, 2023

odeke-em left a comment

coderabbitai bot left a comment

coderabbitai bot left a comment

julienrbrt commented Dec 6, 2023

coderabbitai bot left a comment

coderabbitai bot left a comment

		@@ -134,13 +157,15 @@ enum ProposalStatus {
		// TallyResult defines a standard tally for a governance proposal.
		message TallyResult {

	return OptionEmpty, fmt.Errorf("'%s' is not a valid vote option, available options: yes,option_one/no,option_three/no_with_veto,option_four/abstain,option_two/spam", str)
	return OptionEmpty, fmt.Errorf("'%s' is not a valid vote option, available options: option_one, option_two, option_three, option_four, spam", str)

feat(gov): add proposal types and spam votes #18532

feat(gov): add proposal types and spam votes #18532

Conversation

julienrbrt commented Nov 21, 2023 • edited by coderabbitai bot Loading

Description

Author Checklist

Reviewers Checklist

coderabbitai bot commented Nov 21, 2023 • edited Loading

Walkthrough

Changes

Chat with CodeRabbit Bot (@coderabbitai)

CodeRabbit Commands (invoked as PR comments)

CodeRabbit Configration File (.coderabbit.yaml)

julienrbrt Nov 21, 2023

Choose a reason for hiding this comment

coderabbitai bot left a comment

Choose a reason for hiding this comment

coderabbitai bot left a comment

Choose a reason for hiding this comment

coderabbitai bot Nov 22, 2023

Choose a reason for hiding this comment

coderabbitai bot Nov 22, 2023

Choose a reason for hiding this comment

coderabbitai bot Nov 22, 2023

Choose a reason for hiding this comment

coderabbitai bot Nov 22, 2023

Choose a reason for hiding this comment

coderabbitai bot Nov 22, 2023

Choose a reason for hiding this comment

coderabbitai bot Nov 22, 2023

Choose a reason for hiding this comment

coderabbitai bot Nov 22, 2023

Choose a reason for hiding this comment

odeke-em left a comment

Choose a reason for hiding this comment

coderabbitai bot left a comment

Choose a reason for hiding this comment

coderabbitai bot left a comment

Choose a reason for hiding this comment

julienrbrt commented Dec 6, 2023

coderabbitai bot left a comment

Choose a reason for hiding this comment

coderabbitai bot left a comment

Choose a reason for hiding this comment

julienrbrt commented Nov 21, 2023 •

edited by coderabbitai bot

Loading

coderabbitai bot commented Nov 21, 2023 •

edited

Loading

Chat with CodeRabbit Bot (`@coderabbitai`)

CodeRabbit Configration File (`.coderabbit.yaml`)