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

Add update params to marker module #1991

Merged

Conversation

nullpointer0x00
Copy link
Contributor

@nullpointer0x00 nullpointer0x00 commented May 22, 2024

Description

closes: #XXXX


Before we can merge this PR, please make sure that all the following items have been
checked off. If any of the checklist items are not applicable, please leave them but
write a little note why.

  • Targeted PR against correct branch (see CONTRIBUTING.md)
  • Linked to Github issue with discussion and accepted design OR link to spec that describes this work.
  • Wrote unit and integration tests
  • Updated relevant documentation (docs/) or specification (x/<module>/spec/)
  • Added relevant godoc comments.
  • Added a relevant changelog entry to the Unreleased section in CHANGELOG.md
  • Re-reviewed Files changed in the Github PR explorer
  • Review Codecov Report in the comment section below once CI passes

Summary by CodeRabbit

  • New Features

    • Added UpdateParams endpoint and CLI for the marker module.
    • Added EventMarkerParamsUpdated event to track marker parameter updates.
  • Bug Fixes

    • Fixed validation logic for supply increase by using GetMaxSupply instead of GetParams(ctx).MaxSupply.
  • Documentation

    • Updated Swagger documentation to include MsgUpdateParamsResponse.
  • Tests

    • Added comprehensive test cases for UpdateParams endpoint and marker parameter updates.
    • Enhanced validation tests for Params struct.
  • Refactor

    • Consolidated logic for retrieving maximum supply into GetMaxSupply.
    • Removed deprecated GetMaxTotalSupply function.

Copy link
Contributor

coderabbitai bot commented May 22, 2024

Walkthrough

The recent update introduces a new UpdateParams endpoint and CLI to the marker module in the provenance project. This update spans across various files, including swagger.yaml, proto files, CLI commands, and test cases. It focuses on enabling governance proposals for updating marker module parameters, validating authority, and managing marker parameter events.

Changes

File(s) Change Summary
CHANGELOG.md, client/docs/swagger-ui/swagger.yaml Added UpdateParams endpoint and CLI to marker module. Introduced MsgUpdateParamsResponse definition.
proto/provenance/ibcratelimit/v1/tx.proto Updated comment to reference the ibcratelimit module.
proto/provenance/marker/v1/marker.proto Added EventMarkerParamsUpdated message.
proto/provenance/marker/v1/tx.proto Added UpdateParams endpoint, MsgUpdateParamsRequest, and MsgUpdateParamsResponse message types.
x/marker/client/cli/cli_test.go Added TestUpdateMarkerParamsCmd function for testing marker parameter updates.
x/marker/client/cli/tx.go Introduced NewUpdateMarkerParamsCmd for governance proposals to update marker parameters.
x/marker/keeper/keeper.go Added IsAuthority and ValidateAuthority functions.
x/marker/keeper/marker.go Updated IncreaseSupply function to use k.GetMaxSupply(ctx).
x/marker/keeper/msg_server.go Added UpdateParams function to msgServer struct.
x/marker/keeper/msg_server_test.go Added TestMsgUpdateParamsRequest function with various scenarios.
x/marker/keeper/params.go Removed GetMaxTotalSupply function and consolidated logic into GetMaxSupply.
x/marker/keeper/params_test.go Adjusted test cases for Params struct validation.
x/marker/types/events.go Added NewEventMarkerParamsUpdated function.
x/marker/types/msgs.go Added MsgUpdateParamsRequest to AllRequestMsgs, NewMsgUpdateParamsRequest, and ValidateBasic functions.
x/marker/types/msgs_test.go Added TestMsgUpdateParamsRequestValidateBasic function.
x/marker/types/params.go Removed maxTotalSupply parameter and related validation functions.
x/marker/types/params_test.go Updated test cases for Params struct validation.

🐇 A Rabbit's Ode to Change 🌿

In the realm of code, where markers play,
New params arise, in a bright new way.
Governance reigns, with authority clear,
As tests ensure, there's nothing to fear.
With each new line, the future's bright,
A rabbit's joy, in code's delight. 🌟

Tip

New Features and Improvements

Review Settings

Introduced new personality profiles for code reviews. Users can now select between "Chill" and "Assertive" review tones to tailor feedback styles according to their preferences. The "Assertive" profile posts more comments and nitpicks the code more aggressively, while the "Chill" profile is more relaxed and posts fewer comments.

AST-based Instructions

CodeRabbit offers customizing reviews based on the Abstract Syntax Tree (AST) pattern matching. Read more about AST-based instructions in the documentation.

Community-driven AST-based Rules

We are kicking off a community-driven initiative to create and share AST-based rules. Users can now contribute their AST-based rules to detect security vulnerabilities, code smells, and anti-patterns. Please see the ast-grep-essentials repository for more information.

New Static Analysis Tools

We are continually expanding our support for static analysis tools. We have added support for biome, hadolint, and ast-grep. Update the settings in your .coderabbit.yaml file or head over to the settings page to enable or disable the tools you want to use.

Tone Settings

Users can now customize CodeRabbit to review code in the style of their favorite characters or personalities. Here are some of our favorite examples:

  • Mr. T: "You must talk like Mr. T in all your code reviews. I pity the fool who doesn't!"
  • Pirate: "Arr, matey! Ye must talk like a pirate in all yer code reviews. Yarrr!"
  • Snarky: "You must be snarky in all your code reviews. Snark, snark, snark!"

Revamped Settings Page

We have redesigned the settings page for a more intuitive layout, enabling users to find and adjust settings quickly. This change was long overdue; it not only improves the user experience but also allows our development team to add more settings in the future with ease. Going forward, the changes to .coderabbit.yaml will be reflected in the settings page, and vice versa.

Miscellaneous

  • Turn off free summarization: You can switch off free summarization of PRs opened by users not on a paid plan using the enable_free_tier setting.
  • Knowledge-base scope: You can now set the scope of the knowledge base to either the repository (local) or the organization (global) level using the knowledge_base setting. In addition, you can specify Jira project keys and Linear team keys to limit the knowledge base scope for those integrations.
  • High-level summary placement: You can now customize the location of the high-level summary in the PR description using the high_level_summary_placeholder setting (default @coderabbitai summary).
  • Revamped request changes workflow: You can now configure CodeRabbit to auto-approve or request changes on PRs based on the review feedback using the request_changes_workflow setting.

Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media?

Share
Tips

Chat

There are 3 ways to chat with CodeRabbit:

  • Review comments: Directly reply to a review comment made by CodeRabbit. Example:
    • I pushed a fix in commit <commit_id>.
    • Generate unit testing code for this file.
    • Open a follow-up GitHub issue for this discussion.
  • Files and specific lines of code (under the "Files changed" tab): Tag @coderabbitai in a new review comment at the desired location with your query. Examples:
    • @coderabbitai generate unit testing code for this file.
    • @coderabbitai modularize this function.
  • PR comments: Tag @coderabbitai in a new PR comment to ask questions about the PR branch. For the best results, please provide a very specific query, as very limited context is provided in this mode. Examples:
    • @coderabbitai generate interesting stats about this repository and render them as a table.
    • @coderabbitai show all the console.log statements in this repository.
    • @coderabbitai read src/utils.ts and generate unit testing code.
    • @coderabbitai read the files in the src/scheduler package and generate a class diagram using mermaid and a README in the markdown format.

Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments.

CodeRabbit Commands (invoked as PR comments)

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

Additionally, you can add @coderabbitai ignore anywhere in the PR description to prevent this PR from being reviewed.

CodeRabbit Configration File (.coderabbit.yaml)

  • You can programmatically configure CodeRabbit by adding a .coderabbit.yaml file to the root of your repository.
  • Please see the configuration documentation for more information.
  • 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/schema.v2.json

Documentation and Community

  • Visit our Documentation for detailed information on how to use CodeRabbit.
  • Join our Discord Community to get help, request features, and share feedback.
  • Follow us on X/Twitter for updates and announcements.

@nullpointer0x00 nullpointer0x00 self-assigned this May 22, 2024
@nullpointer0x00 nullpointer0x00 added the marker Marker Module label May 22, 2024
@nullpointer0x00 nullpointer0x00 added this to the v1.19.0 milestone May 22, 2024
@nullpointer0x00 nullpointer0x00 marked this pull request as ready for review May 22, 2024 18:44
@nullpointer0x00 nullpointer0x00 requested a review from a team as a code owner May 22, 2024 18:44
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.

Actionable comments posted: 3

Outside diff range and nitpick comments (6)
x/marker/types/events.go (1)

187-194: Consider using direct string methods for sdkmath.Int and boolean types if available, instead of strconv, for consistency and performance.

CHANGELOG.md (5)

Line range hint 111-357: Consider using a consistent list marker style in Markdown.

- - Bump cosmos-SDK to `v0.50.2` (from `v0.46.13-pio-2`) [#1772](https://github.com/provenance-io/provenance/issues/1772).
+ * Bump cosmos-SDK to `v0.50.2` (from `v0.46.13-pio-2`) [#1772](https://github.com/provenance-io/provenance/issues/1772).

Also applies to: 713-713, 721-721, 766-766, 774-774


Line range hint 820-820: Remove extra blank lines to maintain consistency in document formatting.

- 

Also applies to: 1221-1221, 1238-1238, 1290-1290, 1394-1394


Line range hint 231-231: Consider using Markdown links instead of bare URLs for better readability and navigation.

- https://github.com/provenance-io/provenance/pull/1971
+ [PR 1971](https://github.com/provenance-io/provenance/pull/1971)

Also applies to: 262-262, 361-361, 411-411, 423-423, 439-439, 510-510, 521-521, 529-529, 565-565, 590-590, 602-602, 648-648, 649-649, 650-650, 651-651, 652-652, 699-699, 740-740, 793-793, 906-906


Line range hint 948-948: Remove unnecessary spaces inside emphasis markers.

- * `ibcratelimit` add `UpdateParams` endpoint and deprecate `GovUpdateParams` [#1984](https://github.com/provenance-io/provenance/pull/1984).
+ * `ibcratelimit` add `UpdateParams` endpoint and deprecate `GovUpdateParams` [#1984](https://github.com/provenance-io/provenance/pull/1984).

Line range hint 344-344: Remove unnecessary spaces inside code span elements.

- ` MsgUpdateParamsRequest ` 
+ `MsgUpdateParamsRequest`
Review Details

Configuration used: CodeRabbit UI
Review profile: CHILL

Commits Files that changed from the base of the PR and between 86b2af7 and e8bc9a3.
Files ignored due to path filters (3)
  • x/ibcratelimit/tx.pb.go is excluded by !**/*.pb.go
  • x/marker/types/marker.pb.go is excluded by !**/*.pb.go
  • x/marker/types/tx.pb.go is excluded by !**/*.pb.go
Files selected for processing (18)
  • CHANGELOG.md (1 hunks)
  • client/docs/swagger-ui/swagger.yaml (1 hunks)
  • proto/provenance/ibcratelimit/v1/tx.proto (1 hunks)
  • proto/provenance/marker/v1/marker.proto (1 hunks)
  • proto/provenance/marker/v1/tx.proto (2 hunks)
  • x/marker/client/cli/cli_test.go (1 hunks)
  • x/marker/client/cli/tx.go (3 hunks)
  • x/marker/keeper/keeper.go (2 hunks)
  • x/marker/keeper/marker.go (1 hunks)
  • x/marker/keeper/msg_server.go (1 hunks)
  • x/marker/keeper/msg_server_test.go (1 hunks)
  • x/marker/keeper/params.go (1 hunks)
  • x/marker/keeper/params_test.go (1 hunks)
  • x/marker/types/events.go (2 hunks)
  • x/marker/types/msgs.go (2 hunks)
  • x/marker/types/msgs_test.go (2 hunks)
  • x/marker/types/params.go (5 hunks)
  • x/marker/types/params_test.go (3 hunks)
Files skipped from review due to trivial changes (4)
  • client/docs/swagger-ui/swagger.yaml
  • proto/provenance/ibcratelimit/v1/tx.proto
  • proto/provenance/marker/v1/marker.proto
  • x/marker/keeper/params.go
Additional Context Used
Markdownlint (107)
CHANGELOG.md (107)

111: Expected: asterisk; Actual: dash
Unordered list style


112: Expected: asterisk; Actual: dash
Unordered list style


113: Expected: asterisk; Actual: dash
Unordered list style


114: Expected: asterisk; Actual: dash
Unordered list style


115: Expected: asterisk; Actual: dash
Unordered list style


116: Expected: asterisk; Actual: dash
Unordered list style


117: Expected: asterisk; Actual: dash
Unordered list style


118: Expected: asterisk; Actual: dash
Unordered list style


119: Expected: asterisk; Actual: dash
Unordered list style


120: Expected: asterisk; Actual: dash
Unordered list style


121: Expected: asterisk; Actual: dash
Unordered list style


122: Expected: asterisk; Actual: dash
Unordered list style


123: Expected: asterisk; Actual: dash
Unordered list style


124: Expected: asterisk; Actual: dash
Unordered list style


125: Expected: asterisk; Actual: dash
Unordered list style


126: Expected: asterisk; Actual: dash
Unordered list style


196: Expected: asterisk; Actual: dash
Unordered list style


197: Expected: asterisk; Actual: dash
Unordered list style


198: Expected: asterisk; Actual: dash
Unordered list style


199: Expected: asterisk; Actual: dash
Unordered list style


200: Expected: asterisk; Actual: dash
Unordered list style


201: Expected: asterisk; Actual: dash
Unordered list style


202: Expected: asterisk; Actual: dash
Unordered list style


203: Expected: asterisk; Actual: dash
Unordered list style


204: Expected: asterisk; Actual: dash
Unordered list style


205: Expected: asterisk; Actual: dash
Unordered list style


206: Expected: asterisk; Actual: dash
Unordered list style


207: Expected: asterisk; Actual: dash
Unordered list style


208: Expected: asterisk; Actual: dash
Unordered list style


209: Expected: asterisk; Actual: dash
Unordered list style


210: Expected: asterisk; Actual: dash
Unordered list style


211: Expected: asterisk; Actual: dash
Unordered list style


212: Expected: asterisk; Actual: dash
Unordered list style


213: Expected: asterisk; Actual: dash
Unordered list style


214: Expected: asterisk; Actual: dash
Unordered list style


215: Expected: asterisk; Actual: dash
Unordered list style


216: Expected: asterisk; Actual: dash
Unordered list style


217: Expected: asterisk; Actual: dash
Unordered list style


218: Expected: asterisk; Actual: dash
Unordered list style


219: Expected: asterisk; Actual: dash
Unordered list style


220: Expected: asterisk; Actual: dash
Unordered list style


221: Expected: asterisk; Actual: dash
Unordered list style


222: Expected: asterisk; Actual: dash
Unordered list style


223: Expected: asterisk; Actual: dash
Unordered list style


224: Expected: asterisk; Actual: dash
Unordered list style


225: Expected: asterisk; Actual: dash
Unordered list style


226: Expected: asterisk; Actual: dash
Unordered list style


227: Expected: asterisk; Actual: dash
Unordered list style


256: Expected: asterisk; Actual: dash
Unordered list style


257: Expected: asterisk; Actual: dash
Unordered list style


258: Expected: asterisk; Actual: dash
Unordered list style


335: Expected: asterisk; Actual: dash
Unordered list style


336: Expected: asterisk; Actual: dash
Unordered list style


337: Expected: asterisk; Actual: dash
Unordered list style


338: Expected: asterisk; Actual: dash
Unordered list style


339: Expected: asterisk; Actual: dash
Unordered list style


340: Expected: asterisk; Actual: dash
Unordered list style


341: Expected: asterisk; Actual: dash
Unordered list style


342: Expected: asterisk; Actual: dash
Unordered list style


343: Expected: asterisk; Actual: dash
Unordered list style


344: Expected: asterisk; Actual: dash
Unordered list style


345: Expected: asterisk; Actual: dash
Unordered list style


346: Expected: asterisk; Actual: dash
Unordered list style


347: Expected: asterisk; Actual: dash
Unordered list style


348: Expected: asterisk; Actual: dash
Unordered list style


349: Expected: asterisk; Actual: dash
Unordered list style


350: Expected: asterisk; Actual: dash
Unordered list style


351: Expected: asterisk; Actual: dash
Unordered list style


352: Expected: asterisk; Actual: dash
Unordered list style


353: Expected: asterisk; Actual: dash
Unordered list style


354: Expected: asterisk; Actual: dash
Unordered list style


355: Expected: asterisk; Actual: dash
Unordered list style


356: Expected: asterisk; Actual: dash
Unordered list style


357: Expected: asterisk; Actual: dash
Unordered list style


713: Expected: asterisk; Actual: dash
Unordered list style


721: Expected: asterisk; Actual: dash
Unordered list style


766: Expected: asterisk; Actual: dash
Unordered list style


774: Expected: asterisk; Actual: dash
Unordered list style


820: Expected: 1; Actual: 2
Multiple consecutive blank lines


1221: Expected: 1; Actual: 2
Multiple consecutive blank lines


1238: Expected: 1; Actual: 2
Multiple consecutive blank lines


1290: Expected: 1; Actual: 2
Multiple consecutive blank lines


1394: Expected: 1; Actual: 2
Multiple consecutive blank lines


231: null
Bare URL used


262: null
Bare URL used


361: null
Bare URL used


411: null
Bare URL used


423: null
Bare URL used


439: null
Bare URL used


510: null
Bare URL used


521: null
Bare URL used


529: null
Bare URL used


565: null
Bare URL used


590: null
Bare URL used


602: null
Bare URL used


648: null
Bare URL used


649: null
Bare URL used


650: null
Bare URL used


651: null
Bare URL used


652: null
Bare URL used


699: null
Bare URL used


740: null
Bare URL used


793: null
Bare URL used


906: null
Bare URL used


948: null
Spaces inside emphasis markers


948: null
Spaces inside emphasis markers


344: null
Spaces inside code span elements

Additional comments not posted (14)
x/marker/types/params.go (1)

82-82: These validation functions are marked for removal in a future version. Ensure there is a tracking issue for this task to avoid overlooking it.

Also applies to: 92-92, 102-102

x/marker/types/params_test.go (1)

103-157: The parameter validation tests are comprehensive and cover a variety of scenarios, including edge cases for regex patterns. Good job ensuring robust testing.

x/marker/keeper/keeper.go (1)

207-218: The implementation of authority validation using strings.EqualFold is appropriate for case-insensitive comparison. Ensure that the authority address is correctly set and used throughout the module.

x/marker/types/msgs.go (2)

48-48: Addition of MsgUpdateParamsRequest to AllRequestMsgs is correctly implemented.


727-749: The implementation of NewMsgUpdateParamsRequest and its validation logic in ValidateBasic are correctly done. Ensure that Params.Validate() is implemented appropriately to handle the new parameters.

Verification successful

The Params.Validate method in x/marker/types/params.go correctly validates the UnrestrictedDenomRegex, EnableGovernance, and MaxSupply parameters using appropriate validation functions. The validateRegexParam, validateEnableGovernance, and validateBigIntParam functions are implemented correctly to ensure the parameters are of the expected types and formats.

  • validateRegexParam checks that the regex does not contain anchors and can be compiled.
  • validateEnableGovernance ensures the parameter is a boolean.
  • validateBigIntParam ensures the parameter is of type sdkmath.Int.

No issues were found with the validation logic.

Scripts Executed

The following scripts were executed for the analysis:


Script:

#!/bin/bash
# Description: Verify the implementation of Params.Validate() method.

# Test: Search for the Params.Validate() method implementation. Expect: Proper validation logic for new parameters.
rg --type go $'func \(p Params\) Validate\(\)'

Length of output: 285



Script:

#!/bin/bash
# Extract and display the implementation of Params.Validate method in x/marker/types/params.go
ast-grep --lang go --pattern $'func (p Params) Validate() error' x/marker/types/params.go

Length of output: 393



Script:

#!/bin/bash
# Extract and display the implementation of validateRegexParam function in x/marker/types/params.go
ast-grep --lang go --pattern $'func validateRegexParam($_) error' x/marker/types/params.go

# Check for validation of other parameters in Params.Validate method
rg --type go 'validate' x/marker/types/params.go

Length of output: 1467



Script:

#!/bin/bash
# Extract and display the implementation of validateEnableGovernance function in x/marker/types/params.go
ast-grep --lang go --pattern $'func validateEnableGovernance($_) error' x/marker/types/params.go

# Extract and display the implementation of validateBigIntParam function in x/marker/types/params.go
ast-grep --lang go --pattern $'func validateBigIntParam($_) error' x/marker/types/params.go

Length of output: 925

proto/provenance/marker/v1/tx.proto (2)

79-80: The addition of the UpdateParams method in the Msg service is correctly implemented and follows the protobuf conventions.


467-479: The new message types MsgUpdateParamsRequest and MsgUpdateParamsResponse are well-defined and align with the intended functionality of updating marker module parameters.

x/marker/keeper/msg_server.go (1)

824-838: The implementation of the UpdateParams function in the marker module's message server is correct. It properly handles authority validation, parameter updating, and event emission.

x/marker/keeper/marker.go (1)

349-349: Change to use k.GetMaxSupply(ctx) improves maintainability by consolidating parameter management.

Verification successful

The reference to GetParams(ctx).MaxSupply still exists within the GetMaxSupply function itself, which means the change to use GetMaxSupply(ctx) is correctly implemented. However, the GetMaxSupply function internally still relies on GetParams(ctx).MaxSupply.

  • x/marker/keeper/params.go:
    • GetMaxSupply function still uses GetParams(ctx).MaxSupply.

This indicates that the change to use GetMaxSupply(ctx) is consistent with the intended refactor, but the underlying parameter retrieval hasn't been altered.

Scripts Executed

The following scripts were executed for the analysis:


Script:

#!/bin/bash
# Description: Verify that all references to `GetParams(ctx).MaxSupply` have been replaced with `GetMaxSupply(ctx)`.

# Test: Search for the old function usage. Expect: No occurrences of the old function.
rg --type go $'GetParams\(.*\)\.MaxSupply'

Length of output: 104



Script:

#!/bin/bash
# Description: Locate the exact line and context of `GetParams(ctx).MaxSupply` usage in `x/marker/keeper/params.go`.

# Locate the line number and surrounding lines for context.
rg --type go --context 5 $'GetParams\(.*\)\.MaxSupply' x/marker/keeper/params.go

# Verify the implementation of `GetMaxSupply(ctx)` in the codebase.
ast-grep --lang go --pattern $'func GetMaxSupply($_) $_' 

Length of output: 550

x/marker/types/msgs_test.go (2)

54-54: LGTM! Good to see MsgUpdateParamsRequest included in the GetSigners tests.


1223-1285: LGTM! Comprehensive testing of MsgUpdateParamsRequest.ValidateBasic covering various scenarios.

x/marker/client/cli/tx.go (2)

14-14: Import addition of sdkmath is appropriate for handling mathematical operations on the new max-supply parameter.


1557-1601: The implementation of NewUpdateMarkerParamsCmd is correct and well-structured. It properly handles argument parsing and transaction generation for the governance proposal.

x/marker/keeper/msg_server_test.go (1)

1849-1895: The test cases for UpdateParams are well-structured and cover both successful and failure scenarios effectively.

x/marker/client/cli/cli_test.go Outdated Show resolved Hide resolved
x/marker/client/cli/tx.go Outdated Show resolved Hide resolved
x/marker/keeper/params_test.go Show resolved Hide resolved
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.

Actionable comments posted: 0

Review Details

Configuration used: CodeRabbit UI
Review profile: CHILL

Commits Files that changed from the base of the PR and between e8bc9a3 and 257f198.
Files selected for processing (3)
  • x/marker/client/cli/cli_test.go (1 hunks)
  • x/marker/client/cli/tx.go (3 hunks)
  • x/marker/keeper/params_test.go (2 hunks)
Files skipped from review as they are similar to previous changes (3)
  • x/marker/client/cli/cli_test.go
  • x/marker/client/cli/tx.go
  • x/marker/keeper/params_test.go

@nullpointer0x00 nullpointer0x00 enabled auto-merge (squash) May 23, 2024 14:30
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.

Actionable comments posted: 0

Outside diff range and nitpick comments (5)
CHANGELOG.md (5)

Line range hint 115-361: Consider using a consistent list marker style in Markdown.

- * message #<issue-number>
+ - message #<issue-number>

Line range hint 824-824: Remove consecutive blank lines to maintain a clean and consistent format in the document.

- 

Also applies to: 1225-1225, 1242-1242, 1294-1294, 1398-1398


Line range hint 235-235: Consider using Markdown links instead of bare URLs to improve the readability and presentation of the document.

- https://github.com/provenance-io/provenance/issues/123
+ [Issue #123](https://github.com/provenance-io/provenance/issues/123)

Also applies to: 266-266, 365-365, 415-415, 427-427, 443-443, 514-514, 525-525, 533-533, 569-569, 594-594, 606-606, 652-652, 653-653, 654-654, 655-655, 656-656, 703-703, 744-744, 797-797, 910-910


Line range hint 952-952: Remove unnecessary spaces inside emphasis markers to correct Markdown formatting.

- * this is a test *
+ *this is a test*

Line range hint 348-348: Remove unnecessary spaces inside code span elements to correct Markdown formatting.

- ` code `
+ `code`
Review Details

Configuration used: CodeRabbit UI
Review profile: CHILL

Commits Files that changed from the base of the PR and between 257f198 and 477bf33.
Files selected for processing (2)
  • CHANGELOG.md (1 hunks)
  • x/marker/client/cli/tx.go (3 hunks)
Files skipped from review as they are similar to previous changes (1)
  • x/marker/client/cli/tx.go
Additional Context Used
Markdownlint (107)
CHANGELOG.md (107)

115: Expected: asterisk; Actual: dash
Unordered list style


116: Expected: asterisk; Actual: dash
Unordered list style


117: Expected: asterisk; Actual: dash
Unordered list style


118: Expected: asterisk; Actual: dash
Unordered list style


119: Expected: asterisk; Actual: dash
Unordered list style


120: Expected: asterisk; Actual: dash
Unordered list style


121: Expected: asterisk; Actual: dash
Unordered list style


122: Expected: asterisk; Actual: dash
Unordered list style


123: Expected: asterisk; Actual: dash
Unordered list style


124: Expected: asterisk; Actual: dash
Unordered list style


125: Expected: asterisk; Actual: dash
Unordered list style


126: Expected: asterisk; Actual: dash
Unordered list style


127: Expected: asterisk; Actual: dash
Unordered list style


128: Expected: asterisk; Actual: dash
Unordered list style


129: Expected: asterisk; Actual: dash
Unordered list style


130: Expected: asterisk; Actual: dash
Unordered list style


200: Expected: asterisk; Actual: dash
Unordered list style


201: Expected: asterisk; Actual: dash
Unordered list style


202: Expected: asterisk; Actual: dash
Unordered list style


203: Expected: asterisk; Actual: dash
Unordered list style


204: Expected: asterisk; Actual: dash
Unordered list style


205: Expected: asterisk; Actual: dash
Unordered list style


206: Expected: asterisk; Actual: dash
Unordered list style


207: Expected: asterisk; Actual: dash
Unordered list style


208: Expected: asterisk; Actual: dash
Unordered list style


209: Expected: asterisk; Actual: dash
Unordered list style


210: Expected: asterisk; Actual: dash
Unordered list style


211: Expected: asterisk; Actual: dash
Unordered list style


212: Expected: asterisk; Actual: dash
Unordered list style


213: Expected: asterisk; Actual: dash
Unordered list style


214: Expected: asterisk; Actual: dash
Unordered list style


215: Expected: asterisk; Actual: dash
Unordered list style


216: Expected: asterisk; Actual: dash
Unordered list style


217: Expected: asterisk; Actual: dash
Unordered list style


218: Expected: asterisk; Actual: dash
Unordered list style


219: Expected: asterisk; Actual: dash
Unordered list style


220: Expected: asterisk; Actual: dash
Unordered list style


221: Expected: asterisk; Actual: dash
Unordered list style


222: Expected: asterisk; Actual: dash
Unordered list style


223: Expected: asterisk; Actual: dash
Unordered list style


224: Expected: asterisk; Actual: dash
Unordered list style


225: Expected: asterisk; Actual: dash
Unordered list style


226: Expected: asterisk; Actual: dash
Unordered list style


227: Expected: asterisk; Actual: dash
Unordered list style


228: Expected: asterisk; Actual: dash
Unordered list style


229: Expected: asterisk; Actual: dash
Unordered list style


230: Expected: asterisk; Actual: dash
Unordered list style


231: Expected: asterisk; Actual: dash
Unordered list style


260: Expected: asterisk; Actual: dash
Unordered list style


261: Expected: asterisk; Actual: dash
Unordered list style


262: Expected: asterisk; Actual: dash
Unordered list style


339: Expected: asterisk; Actual: dash
Unordered list style


340: Expected: asterisk; Actual: dash
Unordered list style


341: Expected: asterisk; Actual: dash
Unordered list style


342: Expected: asterisk; Actual: dash
Unordered list style


343: Expected: asterisk; Actual: dash
Unordered list style


344: Expected: asterisk; Actual: dash
Unordered list style


345: Expected: asterisk; Actual: dash
Unordered list style


346: Expected: asterisk; Actual: dash
Unordered list style


347: Expected: asterisk; Actual: dash
Unordered list style


348: Expected: asterisk; Actual: dash
Unordered list style


349: Expected: asterisk; Actual: dash
Unordered list style


350: Expected: asterisk; Actual: dash
Unordered list style


351: Expected: asterisk; Actual: dash
Unordered list style


352: Expected: asterisk; Actual: dash
Unordered list style


353: Expected: asterisk; Actual: dash
Unordered list style


354: Expected: asterisk; Actual: dash
Unordered list style


355: Expected: asterisk; Actual: dash
Unordered list style


356: Expected: asterisk; Actual: dash
Unordered list style


357: Expected: asterisk; Actual: dash
Unordered list style


358: Expected: asterisk; Actual: dash
Unordered list style


359: Expected: asterisk; Actual: dash
Unordered list style


360: Expected: asterisk; Actual: dash
Unordered list style


361: Expected: asterisk; Actual: dash
Unordered list style


717: Expected: asterisk; Actual: dash
Unordered list style


725: Expected: asterisk; Actual: dash
Unordered list style


770: Expected: asterisk; Actual: dash
Unordered list style


778: Expected: asterisk; Actual: dash
Unordered list style


824: Expected: 1; Actual: 2
Multiple consecutive blank lines


1225: Expected: 1; Actual: 2
Multiple consecutive blank lines


1242: Expected: 1; Actual: 2
Multiple consecutive blank lines


1294: Expected: 1; Actual: 2
Multiple consecutive blank lines


1398: Expected: 1; Actual: 2
Multiple consecutive blank lines


235: null
Bare URL used


266: null
Bare URL used


365: null
Bare URL used


415: null
Bare URL used


427: null
Bare URL used


443: null
Bare URL used


514: null
Bare URL used


525: null
Bare URL used


533: null
Bare URL used


569: null
Bare URL used


594: null
Bare URL used


606: null
Bare URL used


652: null
Bare URL used


653: null
Bare URL used


654: null
Bare URL used


655: null
Bare URL used


656: null
Bare URL used


703: null
Bare URL used


744: null
Bare URL used


797: null
Bare URL used


910: null
Bare URL used


952: null
Spaces inside emphasis markers


952: null
Spaces inside emphasis markers


348: null
Spaces inside code span elements

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.

Actionable comments posted: 0

Outside diff range and nitpick comments (5)
CHANGELOG.md (5)

Line range hint 116-362: Consider using a consistent list marker style in Markdown. The asterisk (*) is commonly used for unordered lists instead of the dash (-).

- - Bump cosmos-SDK to `v0.50.2` (from `v0.46.13-pio-2`) [#1772](https://github.com/provenance-io/provenance/issues/1772).
+ * Bump cosmos-SDK to `v0.50.2` (from `v0.46.13-pio-2`) [#1772](https://github.com/provenance-io/provenance/issues/1772).

Also applies to: 718-718, 726-726, 771-771, 779-779


Line range hint 236-236: Consider using Markdown links instead of bare URLs to improve the readability and presentation of the document.

- https://github.com/provenance-io/provenance/issues/1772
+ [Issue #1772](https://github.com/provenance-io/provenance/issues/1772)

Also applies to: 267-267, 366-366, 416-416, 428-428, 444-444, 515-515, 526-526, 534-534, 570-570, 595-595, 607-607, 653-657, 704-704, 745-745, 798-798, 911-911


Line range hint 953-953: Remove unnecessary spaces inside emphasis markers to maintain Markdown formatting consistency.

- * message *PR<pull-request-number>*
+ *message PR<pull-request-number>*

Line range hint 349-349: Ensure there are no spaces inside code span elements to maintain proper Markdown syntax.

- ` MsgUpdateParamsRequest `
+ `MsgUpdateParamsRequest`

Line range hint 825-825: Remove consecutive blank lines to maintain a clean and professional appearance in the document.

- 
-
+ 

Also applies to: 1226-1226, 1243-1243, 1295-1295, 1399-1399

Review Details

Configuration used: CodeRabbit UI
Review profile: CHILL

Commits Files that changed from the base of the PR and between 477bf33 and b455b7f.
Files selected for processing (1)
  • CHANGELOG.md (1 hunks)
Additional Context Used
Markdownlint (107)
CHANGELOG.md (107)

116: Expected: asterisk; Actual: dash
Unordered list style


117: Expected: asterisk; Actual: dash
Unordered list style


118: Expected: asterisk; Actual: dash
Unordered list style


119: Expected: asterisk; Actual: dash
Unordered list style


120: Expected: asterisk; Actual: dash
Unordered list style


121: Expected: asterisk; Actual: dash
Unordered list style


122: Expected: asterisk; Actual: dash
Unordered list style


123: Expected: asterisk; Actual: dash
Unordered list style


124: Expected: asterisk; Actual: dash
Unordered list style


125: Expected: asterisk; Actual: dash
Unordered list style


126: Expected: asterisk; Actual: dash
Unordered list style


127: Expected: asterisk; Actual: dash
Unordered list style


128: Expected: asterisk; Actual: dash
Unordered list style


129: Expected: asterisk; Actual: dash
Unordered list style


130: Expected: asterisk; Actual: dash
Unordered list style


131: Expected: asterisk; Actual: dash
Unordered list style


201: Expected: asterisk; Actual: dash
Unordered list style


202: Expected: asterisk; Actual: dash
Unordered list style


203: Expected: asterisk; Actual: dash
Unordered list style


204: Expected: asterisk; Actual: dash
Unordered list style


205: Expected: asterisk; Actual: dash
Unordered list style


206: Expected: asterisk; Actual: dash
Unordered list style


207: Expected: asterisk; Actual: dash
Unordered list style


208: Expected: asterisk; Actual: dash
Unordered list style


209: Expected: asterisk; Actual: dash
Unordered list style


210: Expected: asterisk; Actual: dash
Unordered list style


211: Expected: asterisk; Actual: dash
Unordered list style


212: Expected: asterisk; Actual: dash
Unordered list style


213: Expected: asterisk; Actual: dash
Unordered list style


214: Expected: asterisk; Actual: dash
Unordered list style


215: Expected: asterisk; Actual: dash
Unordered list style


216: Expected: asterisk; Actual: dash
Unordered list style


217: Expected: asterisk; Actual: dash
Unordered list style


218: Expected: asterisk; Actual: dash
Unordered list style


219: Expected: asterisk; Actual: dash
Unordered list style


220: Expected: asterisk; Actual: dash
Unordered list style


221: Expected: asterisk; Actual: dash
Unordered list style


222: Expected: asterisk; Actual: dash
Unordered list style


223: Expected: asterisk; Actual: dash
Unordered list style


224: Expected: asterisk; Actual: dash
Unordered list style


225: Expected: asterisk; Actual: dash
Unordered list style


226: Expected: asterisk; Actual: dash
Unordered list style


227: Expected: asterisk; Actual: dash
Unordered list style


228: Expected: asterisk; Actual: dash
Unordered list style


229: Expected: asterisk; Actual: dash
Unordered list style


230: Expected: asterisk; Actual: dash
Unordered list style


231: Expected: asterisk; Actual: dash
Unordered list style


232: Expected: asterisk; Actual: dash
Unordered list style


261: Expected: asterisk; Actual: dash
Unordered list style


262: Expected: asterisk; Actual: dash
Unordered list style


263: Expected: asterisk; Actual: dash
Unordered list style


340: Expected: asterisk; Actual: dash
Unordered list style


341: Expected: asterisk; Actual: dash
Unordered list style


342: Expected: asterisk; Actual: dash
Unordered list style


343: Expected: asterisk; Actual: dash
Unordered list style


344: Expected: asterisk; Actual: dash
Unordered list style


345: Expected: asterisk; Actual: dash
Unordered list style


346: Expected: asterisk; Actual: dash
Unordered list style


347: Expected: asterisk; Actual: dash
Unordered list style


348: Expected: asterisk; Actual: dash
Unordered list style


349: Expected: asterisk; Actual: dash
Unordered list style


350: Expected: asterisk; Actual: dash
Unordered list style


351: Expected: asterisk; Actual: dash
Unordered list style


352: Expected: asterisk; Actual: dash
Unordered list style


353: Expected: asterisk; Actual: dash
Unordered list style


354: Expected: asterisk; Actual: dash
Unordered list style


355: Expected: asterisk; Actual: dash
Unordered list style


356: Expected: asterisk; Actual: dash
Unordered list style


357: Expected: asterisk; Actual: dash
Unordered list style


358: Expected: asterisk; Actual: dash
Unordered list style


359: Expected: asterisk; Actual: dash
Unordered list style


360: Expected: asterisk; Actual: dash
Unordered list style


361: Expected: asterisk; Actual: dash
Unordered list style


362: Expected: asterisk; Actual: dash
Unordered list style


718: Expected: asterisk; Actual: dash
Unordered list style


726: Expected: asterisk; Actual: dash
Unordered list style


771: Expected: asterisk; Actual: dash
Unordered list style


779: Expected: asterisk; Actual: dash
Unordered list style


825: Expected: 1; Actual: 2
Multiple consecutive blank lines


1226: Expected: 1; Actual: 2
Multiple consecutive blank lines


1243: Expected: 1; Actual: 2
Multiple consecutive blank lines


1295: Expected: 1; Actual: 2
Multiple consecutive blank lines


1399: Expected: 1; Actual: 2
Multiple consecutive blank lines


236: null
Bare URL used


267: null
Bare URL used


366: null
Bare URL used


416: null
Bare URL used


428: null
Bare URL used


444: null
Bare URL used


515: null
Bare URL used


526: null
Bare URL used


534: null
Bare URL used


570: null
Bare URL used


595: null
Bare URL used


607: null
Bare URL used


653: null
Bare URL used


654: null
Bare URL used


655: null
Bare URL used


656: null
Bare URL used


657: null
Bare URL used


704: null
Bare URL used


745: null
Bare URL used


798: null
Bare URL used


911: null
Bare URL used


953: null
Spaces inside emphasis markers


953: null
Spaces inside emphasis markers


349: null
Spaces inside code span elements

@Taztingo
Copy link
Contributor

LGTM, but you probably want to update the spec with the new included Event.

Taztingo
Taztingo previously approved these changes May 24, 2024
iramiller
iramiller previously approved these changes May 24, 2024
@nullpointer0x00 nullpointer0x00 dismissed stale reviews from iramiller and Taztingo via 8a575b0 May 24, 2024 13:48
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.

Actionable comments posted: 1

Outside diff range and nitpick comments (1)
x/marker/spec/07_events.md (1)

19-19: Add a brief description for the "Marker Params Updated" section in the table of contents for consistency with other entries.

Review Details

Configuration used: CodeRabbit UI
Review profile: CHILL

Commits Files that changed from the base of the PR and between b455b7f and 8a575b0.
Files selected for processing (1)
  • x/marker/spec/07_events.md (2 hunks)
Additional Context Used
Markdownlint (32)
x/marker/spec/07_events.md (32)

6: Expected: 0; Actual: 2
Unordered list indentation


7: Expected: 0; Actual: 2
Unordered list indentation


8: Expected: 0; Actual: 2
Unordered list indentation


9: Expected: 0; Actual: 2
Unordered list indentation


10: Expected: 0; Actual: 2
Unordered list indentation


11: Expected: 0; Actual: 2
Unordered list indentation


12: Expected: 0; Actual: 2
Unordered list indentation


13: Expected: 0; Actual: 2
Unordered list indentation


14: Expected: 0; Actual: 2
Unordered list indentation


15: Expected: 0; Actual: 2
Unordered list indentation


16: Expected: 0; Actual: 2
Unordered list indentation


17: Expected: 0; Actual: 2
Unordered list indentation


18: Expected: 0; Actual: 2
Unordered list indentation


19: Expected: 0; Actual: 2
Unordered list indentation


231: Expected: 0 or 2; Actual: 1
Trailing spaces


21: Expected: 1; Actual: 2
Multiple consecutive blank lines


22: Expected: 1; Actual: 3
Multiple consecutive blank lines


24: Expected: 1; Actual: 0; Above
Headings should be surrounded by blank lines


40: Expected: 1; Actual: 0; Above
Headings should be surrounded by blank lines


62: Expected: 1; Actual: 0; Above
Headings should be surrounded by blank lines


75: Expected: 1; Actual: 0; Above
Headings should be surrounded by blank lines


87: Expected: 1; Actual: 0; Above
Headings should be surrounded by blank lines


99: Expected: 1; Actual: 0; Above
Headings should be surrounded by blank lines


111: Expected: 1; Actual: 0; Above
Headings should be surrounded by blank lines


123: Expected: 1; Actual: 0; Above
Headings should be surrounded by blank lines


136: Expected: 1; Actual: 0; Above
Headings should be surrounded by blank lines


149: Expected: 1; Actual: 0; Above
Headings should be surrounded by blank lines


163: Expected: 1; Actual: 0; Above
Headings should be surrounded by blank lines


178: Expected: 1; Actual: 0; Above
Headings should be surrounded by blank lines


208: Expected: 1; Actual: 0; Above
Headings should be surrounded by blank lines


222: Expected: 1; Actual: 0; Above
Headings should be surrounded by blank lines


232: null
Files should end with a single newline character

x/marker/spec/07_events.md Outdated Show resolved Hide resolved
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.

Actionable comments posted: 1

Review Details

Configuration used: CodeRabbit UI
Review profile: CHILL

Commits Files that changed from the base of the PR and between 8a575b0 and 06e5018.
Files selected for processing (1)
  • x/marker/spec/07_events.md (2 hunks)
Additional Context Used
Markdownlint (32)
x/marker/spec/07_events.md (32)

6: Expected: 0; Actual: 2
Unordered list indentation


7: Expected: 0; Actual: 2
Unordered list indentation


8: Expected: 0; Actual: 2
Unordered list indentation


9: Expected: 0; Actual: 2
Unordered list indentation


10: Expected: 0; Actual: 2
Unordered list indentation


11: Expected: 0; Actual: 2
Unordered list indentation


12: Expected: 0; Actual: 2
Unordered list indentation


13: Expected: 0; Actual: 2
Unordered list indentation


14: Expected: 0; Actual: 2
Unordered list indentation


15: Expected: 0; Actual: 2
Unordered list indentation


16: Expected: 0; Actual: 2
Unordered list indentation


17: Expected: 0; Actual: 2
Unordered list indentation


18: Expected: 0; Actual: 2
Unordered list indentation


19: Expected: 0; Actual: 2
Unordered list indentation


231: Expected: 0 or 2; Actual: 1
Trailing spaces


21: Expected: 1; Actual: 2
Multiple consecutive blank lines


22: Expected: 1; Actual: 3
Multiple consecutive blank lines


24: Expected: 1; Actual: 0; Above
Headings should be surrounded by blank lines


40: Expected: 1; Actual: 0; Above
Headings should be surrounded by blank lines


62: Expected: 1; Actual: 0; Above
Headings should be surrounded by blank lines


75: Expected: 1; Actual: 0; Above
Headings should be surrounded by blank lines


87: Expected: 1; Actual: 0; Above
Headings should be surrounded by blank lines


99: Expected: 1; Actual: 0; Above
Headings should be surrounded by blank lines


111: Expected: 1; Actual: 0; Above
Headings should be surrounded by blank lines


123: Expected: 1; Actual: 0; Above
Headings should be surrounded by blank lines


136: Expected: 1; Actual: 0; Above
Headings should be surrounded by blank lines


149: Expected: 1; Actual: 0; Above
Headings should be surrounded by blank lines


163: Expected: 1; Actual: 0; Above
Headings should be surrounded by blank lines


178: Expected: 1; Actual: 0; Above
Headings should be surrounded by blank lines


208: Expected: 1; Actual: 0; Above
Headings should be surrounded by blank lines


222: Expected: 1; Actual: 0; Above
Headings should be surrounded by blank lines


232: null
Files should end with a single newline character

Additional comments not posted (1)
x/marker/spec/07_events.md (1)

224-232: LGTM! Ensure the file ends with a newline character for proper formatting.

| MaxSupply               | \{value for the max allowed supply\}                |
+ 

x/marker/spec/07_events.md Show resolved Hide resolved
@nullpointer0x00 nullpointer0x00 merged commit 6e3cab1 into main May 24, 2024
27 of 36 checks passed
@nullpointer0x00 nullpointer0x00 deleted the nullpointer0x00/add-update-params-to-marker-module branch May 24, 2024 14:57
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
marker Marker Module
Projects
Development

Successfully merging this pull request may close these issues.

3 participants