Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

test: add counter module #18272

Merged
merged 22 commits into from Oct 30, 2023
Merged

test: add counter module #18272

merged 22 commits into from Oct 30, 2023

Conversation

tac0turtle
Copy link
Member

@tac0turtle tac0turtle commented Oct 26, 2023

Description

This pr adds a small counter module meant to be used for testing when all modules have been spun out into their own go.mods

ref #18290


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

  • included the correct type prefix in the PR title
  • added ! to the type prefix if API or client breaking change
  • targeted the correct branch (see PR Targeting)
  • provided a link to the relevant issue or specification
  • followed the guidelines for building modules
  • included the necessary unit and integration tests
  • added a changelog entry to CHANGELOG.md
  • included comments for documenting Go code
  • updated the relevant documentation or specification
  • reviewed "Files changed" and left comments if necessary
  • run make lint and make test
  • confirmed all CI checks have passed

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)

Summary by CodeRabbit

New Features:

  • Introduced a new application module called "counter" for testing purposes within the Cosmos SDK.
  • Enhanced the "counter" module with new functionalities, including the ability to increase the counter and retrieve the current count.

Refactor:

  • Replaced usage of the "bank" module with the "counter" module across various test files and functions.
  • Updated the "keeper" package to include new fields and functions related to the "counter" module.

Documentation:

  • Added a new README file for the "counter" module, providing an overview of its purpose and usage.
  • Updated the Collections package documentation with revised API details and code examples.

Tests:

  • Modified existing tests to accommodate the shift from the "bank" module to the "counter" module.
  • Introduced a new mock struct for the "AccountKeeper" interface in the testutil package.

Chores:

  • Updated import statements across multiple files to reflect the shift from the "bank" module to the "counter" module.

@coderabbitai
Copy link
Contributor

coderabbitai bot commented Oct 26, 2023

Warning

Rate Limit Exceeded

@tac0turtle has exceeded the limit for the number of files or commits that can be reviewed per hour. Please wait 15 minutes and 38 seconds before requesting another review.

How to resolve this issue?

After the wait time has elapsed, a review can be triggered using the @coderabbitai review command. Alternatively, push new commits to this PR.

We recommend that you space out your commits to avoid hitting the rate limit.

How do rate limits work?

CodeRabbit enforces hourly rate limits for each developer per repository.
Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout.
Please see our FAQ for further information.

Commits Files that changed from the base of the PR and between 879b7bc and f8ef053.

Walkthrough

The changes primarily involve a shift from using the bank module to the counter module in the Cosmos SDK codebase. This includes modifications to import statements, function parameters, variable types, and gRPC queries. Additionally, there are updates to the documentation and test cases to reflect these changes.

Changes

File(s) Summary
api/cosmos/counter/v1/tx.pulsar.go, proto/cosmos/counter/v1/tx.proto Added import statements and modified functions and fields related to the counter module.
client/fuzz_test.go, client/grpc_query_test.go, client/tx/aux_builder_test.go, client/tx/legacy_test.go, client/tx/tx_test.go Removed imports related to bank module, added imports for counter module, and updated function names, parameters, and gRPC queries.
codec/bench_test.go, codec/proto_codec_test.go Replaced usage of bank module with counter module in tests and updated variable types.
collections/README.md Updated documentation and code examples for the Collections package.
testutil/integration/doc.go Added a comment to the integration package.
types/mempool/mempool_test.go Replaced distribution module with counter module and updated function and variable types.
x/circuit/types/keys.go Modified constants in the types package.
x/counter/README.md Added a new file with documentation for the x/counter module.
x/counter/keeper/keeper.go, x/counter/module.go, x/counter/testutil/expected_keepers_mocks.go, x/counter/types/codec.go, x/counter/types/keys.go Introduced new packages and structs, added new fields and functions, and updated return types for the counter module.

Poem

🍂 As autumn leaves fall, we code away, 🍁

From bank to counter, we've made our way. 🚀

With each new commit, a change is born, 🐣

In the world of code, a new day dawns. 🌅

So here's to the devs, in their coding spree, 🥂

Making the Cosmos SDK a sight to see! 🌌

As Halloween 🎃 nears, let's not forget,

In the world of code, there's no threat. 👻

For every bug 🐛 that comes to play,

A CodeRabbit 🐇 is here to save the day! 🦸‍♂️


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 help to get help.
  • @coderabbitai resolve to resolve all the CodeRabbit review comments.

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

@tac0turtle tac0turtle marked this pull request as ready for review October 30, 2023 13:43
@tac0turtle tac0turtle requested a review from a team as a code owner October 30, 2023 13:43
Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Review Status

Actionable comments generated: 3

Configuration used: CodeRabbit UI

Commits Files that changed from the base of the PR and between 393de26 and 43b54ff.
Files ignored due to filter (42)
  • client/v2/go.mod
  • client/v2/go.sum
  • go.mod
  • go.sum
  • simapp/go.mod
  • simapp/go.sum
  • simapp/gomod2nix.toml
  • tests/go.mod
  • tests/go.sum
  • tests/starship/tests/go.mod
  • tests/starship/tests/go.sum
  • x/authz/go.mod
  • x/authz/go.sum
  • x/bank/go.mod
  • x/bank/go.sum
  • x/circuit/go.mod
  • x/circuit/go.sum
  • x/counter/types/tx.pb.go
  • x/distribution/go.mod
  • x/distribution/go.sum
  • x/evidence/go.mod
  • x/evidence/go.sum
  • x/feegrant/go.mod
  • x/feegrant/go.sum
  • x/gov/go.mod
  • x/gov/go.sum
  • x/group/go.mod
  • x/group/go.sum
  • x/mint/go.mod
  • x/mint/go.sum
  • x/nft/go.mod
  • x/nft/go.sum
  • x/params/go.mod
  • x/params/go.sum
  • x/protocolpool/go.mod
  • x/protocolpool/go.sum
  • x/slashing/go.mod
  • x/slashing/go.sum
  • x/staking/go.mod
  • x/staking/go.sum
  • x/upgrade/go.mod
  • x/upgrade/go.sum
Files selected for processing (19)
  • api/cosmos/counter/v1/tx.pulsar.go (2 hunks)
  • client/fuzz_test.go (2 hunks)
  • client/grpc_query_test.go (1 hunks)
  • client/tx/aux_builder_test.go (2 hunks)
  • client/tx/legacy_test.go (2 hunks)
  • client/tx/tx_test.go (8 hunks)
  • codec/bench_test.go (4 hunks)
  • codec/proto_codec_test.go (3 hunks)
  • collections/README.md (6 hunks)
  • proto/cosmos/counter/v1/tx.proto (2 hunks)
  • testutil/integration/doc.go (1 hunks)
  • types/mempool/mempool_test.go (3 hunks)
  • x/circuit/types/keys.go (1 hunks)
  • x/counter/README.md (1 hunks)
  • x/counter/keeper/keeper.go (1 hunks)
  • x/counter/module.go (1 hunks)
  • x/counter/testutil/expected_keepers_mocks.go (1 hunks)
  • x/counter/types/codec.go (1 hunks)
  • x/counter/types/keys.go (1 hunks)
Files skipped from review due to trivial changes (8)
  • client/grpc_query_test.go
  • client/tx/aux_builder_test.go
  • collections/README.md
  • testutil/integration/doc.go
  • x/circuit/types/keys.go
  • x/counter/README.md
  • x/counter/types/codec.go
  • x/counter/types/keys.go
Additional comments: 48
x/counter/testutil/expected_keepers_mocks.go (1)
  • 1-228: This is a generated mock file for testing purposes. It seems to be correctly generated and follows the standard GoMock conventions. Ensure that the methods in the MockAccountKeeper struct match the methods in the AccountKeeper interface. If the interface changes, this mock file should be regenerated to reflect those changes.
proto/cosmos/counter/v1/tx.proto (2)
  • 4-10: The import statement for "amino/amino.proto" has been added. Ensure that this import is necessary and that the package is included in the project dependencies.

  • 20-24: The option "(amino.name) = "cosmos-sdk/increase_counter";" has been added to the message "MsgIncreaseCounter". This seems to be a temporary measure as indicated by the comment "TODO: remove amino". Ensure that this is tracked and removed as planned.

client/tx/legacy_test.go (2)
  • 1-6: The import statements have been updated to include the new counter module and remove the old bank module. Ensure that the counter module is correctly implemented and that it provides all the necessary types and functions that were previously provided by the bank module.

  • 17-17: The msg1 variable has been updated to use the MsgIncreaseCounter type from the counter module. Ensure that all uses of msg1 in the code are compatible with this new type.

api/cosmos/counter/v1/tx.pulsar.go (2)
  • 2-8: > Note: This review was outside of the patch, so it was mapped to the patch with the greatest overlap. Original lines [5-9]

The import statement for "cosmossdk.io/api/amino" has been added. Ensure that this package is being used in the code and is not an unnecessary import. If it's not used, it should be removed to keep the code clean and maintainable.

  • 995-1027: The function "MsgIncreaseCounter" has been modified to include a new parameter "signer". Ensure that all calls to this function throughout the codebase have been updated to match the new signature. Also, the function "MsgIncreaseCounterResponse" has been modified to include a new field "new_count". Make sure that the response handling code is updated to handle this new field.
codec/proto_codec_test.go (3)
  • 16-23: The import statements have been updated to include the new counter module and its types. Ensure that the counter module is correctly implemented and that it provides the necessary functionality that was previously provided by the bank module.

  • 114-117: This test case checks if a typed nil input to the grpcServerEncode function doesn't cause a panic. It's good to see that edge cases are being considered in the tests.

  • 187-195: The test case checks if the GetMsgV1Signers function correctly retrieves the signers from a MsgIncreaseCounter message. It's good to see that the new counter module is being thoroughly tested.

client/fuzz_test.go (3)
  • 10-12: The import paths have been updated to reflect the new counter module. Ensure that the new import paths are correct and the counter module is available at the specified location.

  • 18-21: The function FuzzQueryBalance has been renamed to FuzzQuery. Ensure that all references to this function in the codebase have been updated to reflect the new name.

  • 26-48: The type types.QueryBalanceRequest has been replaced with types.QueryGetCountRequest and the function Balance has been replaced with GetCount in the gRPC query call. Ensure that the GetCount function is implemented in the counter module and that it behaves as expected.

types/mempool/mempool_test.go (5)
  • 10-15: The import statements look fine. No issues found.

  • 18-22: The import of the new counter module is correctly placed. Ensure that the counter module is correctly implemented and available at the specified location.

  • 226-234: The test function TestSampleTxs is updated to use the counter module. Ensure that the unmarshalTx function and msgCounter variable are correctly implemented to work with the counter module.

  • 236-239: The unmarshalTx function is updated to use the counter module. Ensure that the counter.AppModuleBasic{} is correctly implemented and can be used to create a test encoding config.

  • 241-241: The msgCounter variable is updated to represent a counter message. Ensure that the message is correctly formatted and can be used with the counter module.

x/counter/keeper/keeper.go (4)
  • 20-24: The Keeper struct has been updated to include two new fields: event of type event.Service and CountStore of type collections.Item[int64]. Ensure that these new fields are properly initialized and used throughout the codebase.

  • 26-32: The NewKeeper function now accepts an additional parameter em of type event.Service and initializes the new fields event and CountStore. Ensure that all calls to this function throughout the codebase have been updated to match the new signature.

  • 38-50: The GetCount function now returns a *types.QueryGetCountResponse and an error. This is a change from the previous return type. Ensure that all calls to this function throughout the codebase have been updated to handle the new return type.

  • 56-82: The IncreaseCount function now returns a *types.MsgIncreaseCountResponse and an error. This is a change from the previous return type. Ensure that all calls to this function throughout the codebase have been updated to handle the new return type.

codec/bench_test.go (6)
  • 10-16: The import statements have been updated to include the new counter module and its types. Ensure that the paths are correct and the module is properly installed.

  • 19-26: The msgCounterWrapper struct and its GetSigners method have been updated to use the Signer field from the MsgIncreaseCounter message. This change is consistent with the switch from the bank module to the counter module.

  • 28-35: In the BenchmarkLegacyGetSigners function, the msgCounterWrapper is being used with the MsgIncreaseCounter message. This is consistent with the switch from the bank module to the counter module.

  • 43-51: In the BenchmarkProtoreflectGetSigners function, the MsgIncreaseCounter message from the counter module is being used. This is consistent with the switch from the bank module to the counter module.

  • 61-69: In the BenchmarkProtoreflectGetSignersWithUnmarshal function, the MsgIncreaseCounter message from the counter module is being used. This is consistent with the switch from the bank module to the counter module.

  • 82-90: In the BenchmarkProtoreflectGetSignersDynamicpb function, the MsgIncreaseCounter message from the counter module is being used. This is consistent with the switch from the bank module to the counter module.

client/tx/tx_test.go (8)
  • 9-14: The import statements look fine. No changes needed.

  • 24-28: The import statement for the counter module has been added correctly.

  • 126-132: The MsgIncreaseCounter message is being created and used correctly. Ensure that the Signer field is being set to a valid account address and that the Count field is being set to an appropriate value.

  • 142-148: The MsgIncreaseCounter message is being created and used correctly. Ensure that the Signer field is being set to a valid account address and that the Count field is being set to an appropriate value.

  • 161-167: The MsgIncreaseCounter message is being created and used correctly. Ensure that the Signer field is being set to a valid account address and that the Count field is being set to an appropriate value.

  • 204-210: The MsgIncreaseCounter message is being created and used correctly. Ensure that the Signer field is being set to a valid account address and that the Count field is being set to an appropriate value.

  • 255-262: The MsgIncreaseCounter message is being created and used correctly. Ensure that the Signer field is being set to a valid account address and that the Count field is being set to an appropriate value.

  • 403-410: The MsgIncreaseCounter message is being created and used correctly. Ensure that the Signer field is being set to a valid account address and that the Count field is being set to an appropriate value.

x/counter/module.go (12)
  • 1-19: The import statements look fine. Ensure that all the imported packages are being used in the code. Also, make sure that the versions of these packages are compatible with each other.

  • 24-29: The AppModule struct is correctly implementing the module.AppModuleBasic, appmodule.AppModule, and appmodule.HasServices interfaces. This is verified by the blank variable declarations.

  • 31-34: The AppModuleBasic struct is defined with a cdc field of type codec.Codec. Ensure that this field is used appropriately in the methods of this struct.

  • 36-48: The methods Name, RegisterLegacyAminoCodec, RegisterGRPCGatewayRoutes, and RegisterInterfaces are defined for the AppModuleBasic struct. Ensure that these methods are used correctly in the application.

  • 50-55: The AppModule struct is defined with an embedded AppModuleBasic and a keeper field of type keeper.Keeper. Ensure that these fields are used appropriately in the methods of this struct.

  • 57-68: The methods IsOnePerModuleType, IsAppModule, and RegisterServices are defined for the AppModule struct. Ensure that these methods are used correctly in the application.

  • 70-76: The NewAppModule function is defined to create a new AppModule object. Ensure that this function is used correctly in the application.

  • 78-80: The ConsensusVersion method is defined for the AppModule struct. Ensure that this method is used correctly in the application.

  • 81-86: The init function is defined to register the modulev1.Module and ProvideModule function. Ensure that these are used correctly in the application.

  • 88-95: The ModuleInputs struct is defined with several fields. Ensure that these fields are used appropriately in the methods of this struct.

  • 97-102: The ModuleOutputs struct is defined with a Keeper field of type keeper.Keeper and a Module field of type appmodule.AppModule. Ensure that these fields are used appropriately in the methods of this struct.

  • 104-113: The ProvideModule function is defined to create a new ModuleOutputs object. Ensure that this function is used correctly in the application.

Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Review Status

Actionable comments generated: 5

Configuration used: CodeRabbit UI

Commits Files that changed from the base of the PR and between 393de26 and 43b54ff.
Files ignored due to filter (42)
  • client/v2/go.mod
  • client/v2/go.sum
  • go.mod
  • go.sum
  • simapp/go.mod
  • simapp/go.sum
  • simapp/gomod2nix.toml
  • tests/go.mod
  • tests/go.sum
  • tests/starship/tests/go.mod
  • tests/starship/tests/go.sum
  • x/authz/go.mod
  • x/authz/go.sum
  • x/bank/go.mod
  • x/bank/go.sum
  • x/circuit/go.mod
  • x/circuit/go.sum
  • x/counter/types/tx.pb.go
  • x/distribution/go.mod
  • x/distribution/go.sum
  • x/evidence/go.mod
  • x/evidence/go.sum
  • x/feegrant/go.mod
  • x/feegrant/go.sum
  • x/gov/go.mod
  • x/gov/go.sum
  • x/group/go.mod
  • x/group/go.sum
  • x/mint/go.mod
  • x/mint/go.sum
  • x/nft/go.mod
  • x/nft/go.sum
  • x/params/go.mod
  • x/params/go.sum
  • x/protocolpool/go.mod
  • x/protocolpool/go.sum
  • x/slashing/go.mod
  • x/slashing/go.sum
  • x/staking/go.mod
  • x/staking/go.sum
  • x/upgrade/go.mod
  • x/upgrade/go.sum
Files selected for processing (19)
  • api/cosmos/counter/v1/tx.pulsar.go (2 hunks)
  • client/fuzz_test.go (2 hunks)
  • client/grpc_query_test.go (1 hunks)
  • client/tx/aux_builder_test.go (2 hunks)
  • client/tx/legacy_test.go (2 hunks)
  • client/tx/tx_test.go (8 hunks)
  • codec/bench_test.go (4 hunks)
  • codec/proto_codec_test.go (3 hunks)
  • collections/README.md (6 hunks)
  • proto/cosmos/counter/v1/tx.proto (2 hunks)
  • testutil/integration/doc.go (1 hunks)
  • types/mempool/mempool_test.go (3 hunks)
  • x/circuit/types/keys.go (1 hunks)
  • x/counter/README.md (1 hunks)
  • x/counter/keeper/keeper.go (1 hunks)
  • x/counter/module.go (1 hunks)
  • x/counter/testutil/expected_keepers_mocks.go (1 hunks)
  • x/counter/types/codec.go (1 hunks)
  • x/counter/types/keys.go (1 hunks)
Files skipped from review due to trivial changes (8)
  • client/grpc_query_test.go
  • client/tx/aux_builder_test.go
  • client/tx/legacy_test.go
  • collections/README.md
  • testutil/integration/doc.go
  • x/circuit/types/keys.go
  • x/counter/README.md
  • x/counter/types/keys.go
Additional comments: 39
x/counter/types/codec.go (1)
  • 1-16: The new package types and the function RegisterInterfaces seem to be correctly implemented. The function registers the MsgIncreaseCounter message and the message service descriptor as expected. Ensure that the MsgIncreaseCounter message and the _Msg_serviceDesc are correctly defined and implemented elsewhere in the codebase. Also, make sure that the RegisterInterfaces function is called during the initialization of the application to register the interfaces with the Cosmos SDK.
proto/cosmos/counter/v1/tx.proto (2)
  • 4-10: The import statement for "amino/amino.proto" has been added. Ensure that the Amino library is being used correctly and that it is compatible with the rest of the codebase. Also, ensure that the Amino library is maintained and secure to use.

  • 21-24: The option "(amino.name) = "cosmos-sdk/increase_counter";" has been added to the message "MsgIncreaseCounter". This seems to be a temporary measure as indicated by the comment "TODO: remove amino". It would be good to clarify the timeline and plan for this removal.

codec/proto_codec_test.go (3)
  • 13-24: The import statements have been updated to include the new counter module and remove the old bank module. Ensure that the new import paths are correct and that all dependencies for the counter module are properly installed.

  • 111-117: The variable v has been changed from *banktypes.QueryBalanceResponse to *countertypes.QueryGetCountRequest. Ensure that this change is consistent with the rest of the codebase and that all instances of QueryBalanceResponse have been replaced with QueryGetCountRequest where necessary.

  • 187-195: The variables msgSendV1 and msgSendV2 have been changed from *banktypes.MsgSend and *bankv1beta1.MsgSend to *countertypes.MsgIncreaseCounter and *counterv1.MsgIncreaseCounter respectively. Ensure that this change is consistent with the rest of the codebase and that all instances of MsgSend have been replaced with MsgIncreaseCounter where necessary.

client/fuzz_test.go (5)
  • 10-11: The import paths have been updated to reflect the new counter module. Ensure that the new import paths are correct and the counter module is properly installed.

  • 18-21: The function FuzzQueryBalance has been renamed to FuzzQuery. Ensure that all calls to this function throughout the codebase have been updated to match the new name.

  • 29-30: The type types.QueryBalanceRequest has been replaced with types.QueryGetCountRequest. Ensure that this change is consistent with the new counter module's API.

  • 33-48: The function Balance has been replaced with GetCount in the gRPC query call. Ensure that this change is consistent with the new counter module's API.

  • 51-56: The function FuzzQuery has been updated to use the new fuzzSuite struct. Ensure that this change is consistent with the new counter module's API.

x/counter/keeper/keeper.go (4)
  • 20-24: The Keeper struct has been updated to include two new fields: event and CountStore. Ensure that these fields are properly initialized and used throughout the codebase.

  • 26-32: The NewKeeper function has been updated to include a new parameter em of type event.Service. This function now initializes the new fields in the Keeper struct. Ensure that all calls to this function throughout the codebase have been updated to match the new signature.

  • 38-50: The GetCount function now returns a *types.QueryGetCountResponse and an error. This is a change from the previous version of the function. Ensure that all calls to this function throughout the codebase have been updated to handle the new return types.

  • 56-82: The IncreaseCount function now returns a *types.MsgIncreaseCountResponse and an error. This is a change from the previous version of the function. Ensure that all calls to this function throughout the codebase have been updated to handle the new return types.

types/mempool/mempool_test.go (4)
  • 10-15: The import statements look fine. Ensure that all the imported packages are used in the code.

  • 18-22: The import statement for the counter module has been added. Ensure that the counter module is used in the code.

  • 226-234: The unmarshalTx function is used to unmarshal the msgCounter byte array into a sdk.Tx type. Ensure that the msgCounter byte array is correctly formatted and can be unmarshalled without errors.

  • 237-239: The unmarshalTx function uses the counter.AppModuleBasic{} to create a test encoding configuration. Ensure that the counter.AppModuleBasic{} is correctly implemented and can be used to create a test encoding configuration.

241:
The msgCounter byte array is used to test the unmarshalTx function. Ensure that the msgCounter byte array is correctly formatted and can be unmarshalled into a sdk.Tx type.

client/tx/tx_test.go (7)
  • 24-28: The import statement for the counter module has been added. Ensure that the module is correctly installed and available in the project's dependencies.

  • 128-132: The MsgIncreaseCounter message from the counter module is being used. Ensure that the Signer and Count fields are correctly set and that the message is correctly processed in the counter module.

  • 142-148: > Note: This review was outside of the patch, so it was mapped to the patch with the greatest overlap. Original lines [145-149]

The MsgIncreaseCounter message from the counter module is being used. Ensure that the Signer and Count fields are correctly set and that the message is correctly processed in the counter module.

  • 163-167: The MsgIncreaseCounter message from the counter module is being used. Ensure that the Signer and Count fields are correctly set and that the message is correctly processed in the counter module.

  • 206-210: The MsgIncreaseCounter message from the counter module is being used. Ensure that the Signer and Count fields are correctly set and that the message is correctly processed in the counter module.

  • 255-262: The MsgIncreaseCounter message from the counter module is being used. Ensure that the Signer and Count fields are correctly set and that the message is correctly processed in the counter module.

  • 403-410: The MsgIncreaseCounter message from the counter module is being used. Ensure that the Signer and Count fields are correctly set and that the message is correctly processed in the counter module.

codec/bench_test.go (6)
  • 10-16: Ensure that the new import paths are correct and the counter module is properly installed in your environment.

  • 19-26: The msgCounterWrapper struct and its GetSigners method seem to be correctly implemented. However, ensure that the Signer field in MsgIncreaseCounter is always a valid Bech32 address, as the GetSigners method assumes this without error checking.

  • 28-35: In the BenchmarkLegacyGetSigners function, the msgCounterWrapper is correctly initialized and used.

  • 43-51: In the BenchmarkProtoreflectGetSigners function, the MsgIncreaseCounter message is correctly initialized and used.

  • 61-69: In the BenchmarkProtoreflectGetSignersWithUnmarshal function, the MsgIncreaseCounter message is correctly initialized and used.

  • 82-90: In the BenchmarkProtoreflectGetSignersDynamicpb function, the MsgIncreaseCounter message is correctly initialized and used.

x/counter/testutil/expected_keepers_mocks.go (1)
  • 1-228: This is a generated mock file for testing purposes. It seems to be correctly generated and follows the standard GoMock conventions. The methods in the MockAccountKeeper struct match the methods in the AccountKeeper interface, and the MockAccountKeeperMockRecorder struct provides a way to set up expectations for those methods in tests. The NewMockAccountKeeper function correctly creates a new instance of MockAccountKeeper. The EXPECT method returns the recorder for setting up expectations. The mock methods correctly delegate to the Call method of the gomock.Controller, and the recorder methods correctly record calls with the RecordCallWithMethodType method of the gomock.Controller. The ValidatePermissions method has been correctly added to the AccountKeeper interface and the corresponding mock and recorder methods have been added to the MockAccountKeeper and MockAccountKeeperMockRecorder structs.
x/counter/module.go (4)
  • 1-19: Ensure that all the imported packages are used in the code. Unused imports can lead to compilation errors in Go.

  • 64-68: The RegisterServices function registers the MsgServer and QueryServer with the keeper instance. Ensure that the keeper instance is properly initialized before this function is called.

  • 71-76: The NewAppModule function creates a new AppModule instance. Ensure that the cdc and keeper parameters are properly initialized before this function is called.

  • 104-113: The ProvideModule function creates a new Keeper and AppModule instance. Ensure that the StoreService and EventManager fields in the ModuleInputs struct are properly initialized before this function is called.

api/cosmos/counter/v1/tx.pulsar.go (2)
  • 5-8: The import statement for "cosmossdk.io/api/amino" has been added. Ensure that this package is used in the code and is not an unnecessary import. If it's not used, it should be removed to keep the code clean and maintainable.

  • 998-1008: The function "MsgIncreaseCounter" has been modified to include a new parameter "signer". Ensure that all calls to this function throughout the codebase have been updated to match the new signature. Also, the function "MsgIncreaseCounterResponse" has been modified to include a new field "new_count". Make sure that this new field is handled properly in the function's usage.

fz.Require().NoError(err)
f.Add(bz)

// 2. Now fuzz it and ensure that we don't get any panics.
ctx := context.Background()
// ctx := context.Background()
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The parameter ctx in the function FuzzQuery has been commented out. If this context is not needed, it should be removed. If it is needed, it should be uncommented and used appropriately.

- // ctx := context.Background()
+ ctx := context.Background()

Commitable suggestion (Beta)
Suggested change
// ctx := context.Background()
ctx := context.Background()

x/counter/keeper/keeper.go Outdated Show resolved Hide resolved
x/counter/module.go Outdated Show resolved Hide resolved
x/counter/module.go Show resolved Hide resolved
x/counter/module.go Show resolved Hide resolved
@github-prbot github-prbot requested review from a team, facundomedica and julienrbrt and removed request for a team October 30, 2023 13:56
@@ -17,6 +18,7 @@ service Msg {

// MsgIncreaseCounter defines a count Msg service counter.
message MsgIncreaseCounter {
option (amino.name) = "cosmos-sdk/increase_counter"; // TODO: remove amino
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

aux builder test fails without this but its not required anymore, ill look into the test to see if we can remove this here

Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Review Status

Actionable comments generated: 1

Configuration used: CodeRabbit UI

Commits Files that changed from the base of the PR and between 43b54ff and 879b7bc.
Files selected for processing (3)
  • client/fuzz_test.go (2 hunks)
  • x/counter/keeper/keeper.go (1 hunks)
  • x/counter/module.go (1 hunks)
Additional comments: 14
client/fuzz_test.go (4)
  • 10-11: The import paths have been updated to include the new counter module and remove the old bank module. Ensure that the new import paths are correct and that the counter module is correctly implemented in the codebase.

  • 18-21: The function FuzzQuery has been updated to use the new counter module. Ensure that all calls to this function throughout the codebase have been updated to match the new signature.

  • 26-48: The marshaling and unmarshaling of different request types have been changed from types.QueryBalanceRequest to types.QueryGetCountRequest. The gRPC query function has been changed from fz.bankClient.Balance to fz.counterClient.GetCount. Ensure that these changes are correct and that the new request types and gRPC query function are correctly implemented in the codebase.

  • 51-56: The function FuzzQuery has been updated to use the new counter module. Ensure that all calls to this function throughout the codebase have been updated to match the new signature.

x/counter/module.go (5)
  • 35-36: As per the previous discussion, the RegisterLegacyAminoCodec function is kept as boilerplate and will be addressed in a separate issue.

  • 38-39: As per the previous discussion, the RegisterGRPCGatewayRoutes function is kept as boilerplate and will be addressed in a separate issue.

  • 59-63: The RegisterServices function correctly registers the MsgServer and QueryServer with the keeper. This is a good practice as it allows the module to handle messages and queries.

  • 66-71: The NewAppModule function correctly initializes a new AppModule with the provided keeper. This is a good practice as it allows the module to manage state and handle messages and queries.

  • 99-108: The ProvideModule function correctly initializes a new Keeper and AppModule and returns them in a ModuleOutputs struct. This is a good practice as it allows the module to manage state and handle messages and queries.

x/counter/keeper/keeper.go (5)
  • 1-16: The import statements look good and are well organized.

  • 20-24: The Keeper struct has been updated to include new fields event and CountStore. Ensure that these fields are used correctly throughout the codebase.

  • 26-32: The NewKeeper function has been updated to include a new parameter em and initialize the new event field. Ensure that all calls to this function throughout the codebase have been updated to match the new signature.

  • 38-50: The GetCount function now returns a *types.QueryGetCountResponse and an error. This is a breaking change and should be communicated to all users of this function.

  • 56-81: The IncreaseCount function now returns a *types.MsgIncreaseCountResponse and an error. This is a breaking change and should be communicated to all users of this function.

Comment on lines +70 to +76
if err := k.event.EventManager(ctx).EmitKV(
ctx,
"increase_counter",
event.Attribute{Key: "signer", Value: msg.Signer},
event.Attribute{Key: "new count", Value: fmt.Sprint(num + msg.Count)}); err != nil {
return nil, err
}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The event emission logic seems to be correct. However, it's a good practice to define event names and attribute keys as constants to avoid typos and improve maintainability.

+ const EventIncreaseCounter = "increase_counter"
+ const AttributeSigner = "signer"
+ const AttributeNewCount = "new count"

- "increase_counter"
+ EventIncreaseCounter

- "signer"
+ AttributeSigner

- "new count"
+ AttributeNewCount

Commitable suggestion (Beta)
Suggested change
if err := k.event.EventManager(ctx).EmitKV(
ctx,
"increase_counter",
event.Attribute{Key: "signer", Value: msg.Signer},
event.Attribute{Key: "new count", Value: fmt.Sprint(num + msg.Count)}); err != nil {
return nil, err
}
const (
EventIncreaseCounter = "increase_counter"
AttributeSigner = "signer"
AttributeNewCount = "new count"
)
if err := k.event.EventManager(ctx).EmitKV(
ctx,
EventIncreaseCounter,
event.Attribute{Key: AttributeSigner, Value: msg.Signer},
event.Attribute{Key: AttributeNewCount, Value: fmt.Sprint(num + msg.Count)},
); err != nil {
return nil, err
}

@sonarcloud
Copy link

sonarcloud bot commented Oct 30, 2023

[Cosmos SDK - Distribution] Kudos, SonarCloud Quality Gate passed!    Quality Gate passed

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 0 Code Smells

No Coverage information No Coverage information
No Duplication information No Duplication information

@julienrbrt julienrbrt changed the title tests: add counter module test: add counter module Oct 30, 2023
Copy link
Member

@julienrbrt julienrbrt left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lgtm!

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If the dep on distribution and protocolpool is removed, we can remove those replaces from all other go.mod

@@ -28,34 +26,31 @@ func (fz *fuzzSuite) FuzzQueryBalance(f *testing.F) {
fz.Require().Equal("hello", testRes.Message)

// 1. Generate some seeds.
bz, err := fz.cdc.Marshal(&types.QueryBalanceRequest{
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sadly this won't give the fuzzer good direction and it'll take a long while to figure out how a request will look like. Perhaps this is temporary?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

we can move the fuzzing into the bank module but cant keep it at the root level

@tac0turtle tac0turtle added this pull request to the merge queue Oct 30, 2023
Merged via the queue into main with commit 44db964 Oct 30, 2023
80 of 81 checks passed
@tac0turtle tac0turtle deleted the marko/counter branch October 30, 2023 19:09
atheeshp pushed a commit that referenced this pull request Nov 1, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants