-
Notifications
You must be signed in to change notification settings - Fork 39
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
Enhance the config commands and change some defaults. #1968
Conversation
…ing addresses with the wrong hrp for other tests. This doesn't show up if running tests one file at a time, but causes panics when all the tests in that package are run together.
…were using fields that no longer exist.
…he configs for entries, possibly outputting multiple.
…command. Deprecate 'tendermint' and 'tm' as options for 'get' and 'changed', and add 'cometbft', 'comet', and 'cmt' as options.
…date CONTRIBUTING and SECURITY for the change from tendermint to cometbft.
…t a previous value in order to identify fields that were added, removed, renamed, or have new default values.
…block to sync. Make the client config defaults public variables (instead of private consts) so we can update them dynamically if needed.
… for testnets. Add/Update unit tests for the broadcast mode and keyring backend changes, as well as for the pre-upgrade command changes."
… interceptor so that it can get used when loading the files.
…eld matches and loose matches.
…up with the wrong default keyring backend.
WalkthroughThe recent updates focus on replacing dependencies and references from Tendermint to CometBFT across various files. This includes modifying configuration settings, variable names, comments, and error messages to align with the new dependency. Additionally, there are improvements in configuration commands, default settings, and minor formatting corrections. Changes
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (invoked as PR comments)
Additionally, you can add CodeRabbit Configration File (
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 3
Out of diff range and nitpick comments (18)
CONTRIBUTING.md (11)
Line range hint
3-4
: Correct the Markdown link syntax for "Cosmos SDK".- the [Cosmos SDK]](https://github.com/cosmos/cosmos-sdk). + the [Cosmos SDK](https://github.com/cosmos/cosmos-sdk).
Line range hint
4-4
: Consider simplifying "source code outside of the/x/
folder" to "code outside the/x/
folder" to avoid redundancy.- The source code outside of the `/x/` folder is largely based on a reference implementation of this SDK. + The code outside the `/x/` folder is largely based on a reference implementation of this SDK.
Line range hint
30-30
: Clarify the sentence starting with "PR" by adding a verb or rephrasing for better readability.- 4. PR the .x Branch Back to Main + 4. Submit the .x Branch Back to Main as a PR
Line range hint
46-46
: Add a comma before 'and' for grammatical correctness.- of nobody has been assigned for the issue and you would like to work on it, + of nobody has been assigned for the issue, and you would like to work on it,
Line range hint
61-61
: Replace "very small" with "minor" to avoid using the over-used intensifier 'very'.- Note that for very small or blatantly obvious problems (such as typos) it is + Note that for minor or blatantly obvious problems (such as typos) it is
Line range hint
71-71
: Add a comma after 'Additionally' for better readability.- Additionally please ensure that your code is lint compliant by running `make lint`. + Additionally, please ensure that your code is lint compliant by running `make lint`.
Line range hint
78-78
: Add a comma after 'Ideally' for clarity.- Ideally each PR addresses only a single issue. + Ideally, each PR addresses only a single issue.
Line range hint
80-80
: Correct the grammatical error by changing "a separate PRs" to "separate PRs".- code refactoring and cleanup should be submitted as a separate PRs from bugfixes/feature-additions. + code refactoring and cleanup should be submitted as separate PRs from bugfixes/feature-additions.
Line range hint
91-91
: Remove the duplicated word 'must'.- It must must pass all required Github action checks. + It must pass all required Github action checks.
Line range hint
103-103
: Add a comma after 'PRs' for clarity.- When reviewing PRs please use the following review explanations: + When reviewing PRs, please use the following review explanations:
Line range hint
114-114
: Consider rephrasing "surface level" to "preliminary" for a more formal tone.- If you are only making "surface level" reviews, submit any notes as `Comments` + If you are only making preliminary reviews, submit any notes as `Comments`cmd/provenanced/config/manager.go (1)
Line range hint
204-224
: Refactor verbose logging into a separate function to reduce code duplication.- if verbose { - cmd.Printf("Writing cometbft config to: %s ... ", confFile) - } - cmtconfig.WriteConfigFile(confFile, cmtConfig) - if verbose { - cmd.Printf("Done.\n") - } + logVerbose(cmd, verbose, "Writing cometbft config to: %s ... ", confFile) + cmtconfig.WriteConfigFile(confFile, cmtConfig) + logVerbose(cmd, verbose, "Done.\n")CHANGELOG.md (6)
Line range hint
86-331
: Convert list markers from dashes to asterisks for consistency.- * message #<issue-number> + * message #<issue-number>
Line range hint
205-205
: Avoid using bare URLs. Provide a descriptive hyperlink text.- https://github.com/provenance-io/provenance/issues/205 + [Issue #205](https://github.com/provenance-io/provenance/issues/205)
Line range hint
236-236
: Avoid using bare URLs. Provide a descriptive hyperlink text.- https://github.com/provenance-io/provenance/issues/236 + [Issue #236](https://github.com/provenance-io/provenance/issues/236)
Line range hint
794-794
: Remove consecutive blank lines to maintain a clean and professional format.-
Line range hint
922-922
: Remove spaces inside emphasis markers to correct the markdown formatting.- * Created default tendermint config * + *Created default tendermint config*
Line range hint
318-318
: Remove spaces inside code span elements to correct the markdown formatting.- ` MsgAddMarkerRequest ` + `MsgAddMarkerRequest`
Review Details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (28)
- .github/workflows/test.yml (2 hunks)
- .golangci.yml (1 hunks)
- CHANGELOG.md (1 hunks)
- CONTRIBUTING.md (1 hunks)
- Makefile (2 hunks)
- SECURITY.md (2 hunks)
- app/app.go (1 hunks)
- app/test_helpers.go (1 hunks)
- cmd/provenanced/cmd/config.go (19 hunks)
- cmd/provenanced/cmd/config_test.go (26 hunks)
- cmd/provenanced/cmd/docgen_test.go (1 hunks)
- cmd/provenanced/cmd/init.go (4 hunks)
- cmd/provenanced/cmd/pre_upgrade.go (3 hunks)
- cmd/provenanced/cmd/pre_upgrade_test.go (10 hunks)
- cmd/provenanced/cmd/root.go (1 hunks)
- cmd/provenanced/cmd/testnet.go (1 hunks)
- cmd/provenanced/cmd/testnet_test.go (2 hunks)
- cmd/provenanced/config/client.go (4 hunks)
- cmd/provenanced/config/filenames.go (2 hunks)
- cmd/provenanced/config/interceptor.go (2 hunks)
- cmd/provenanced/config/manager.go (17 hunks)
- cmd/provenanced/config/manager_test.go (14 hunks)
- cmd/provenanced/config/reflector.go (1 hunks)
- cmd/provenanced/config/reflector_test.go (5 hunks)
- cmd/provenanced/config/toml.go (2 hunks)
- internal/handlers/aggregate_events_test.go (1 hunks)
- x/quarantine/client/testutil/query_test.go (4 hunks)
- x/quarantine/client/testutil/tx_test.go (2 hunks)
Files skipped from review due to trivial changes (9)
- .github/workflows/test.yml
- app/app.go
- app/test_helpers.go
- cmd/provenanced/cmd/docgen_test.go
- cmd/provenanced/cmd/root.go
- cmd/provenanced/cmd/testnet.go
- cmd/provenanced/config/toml.go
- x/quarantine/client/testutil/query_test.go
- x/quarantine/client/testutil/tx_test.go
Additional Context Used
LanguageTool (34)
CONTRIBUTING.md (30)
Near line 3: If this is a compound adjective that modifies the following noun, use a hyphen.
Context: ...repository is built on the work of many open source projects including the [Cosmos SDK]](ht...
Near line 4: Unpaired symbol: ‘’ seems to be missing
Context: ...urce projects including the [Cosmos SDK]. ...
Near line 4: This phrase is redundant. Consider using “outside”.
Context: ...om/cosmos/cosmos-sdk). The source code outside of the/x/
folder is largely based on a ...
Near line 30: This sentence should probably be started with a verb instead of the noun ‘PR’. If not, consider inserting a comma for better clarity.
Context: ...create-the-new-version-tag) - [4. PR the .x Branch Back to Main](#4-pr-the-x...
Near line 46: Use a comma before ‘and’ if it connects two independent clauses (unless they are closely connected and short).
Context: ...f nobody has been assigned for the issue and you would like to work on it, mak...
Near line 61: As an alternative to the over-used intensifier ‘very’, consider replacing this phrase.
Context: ...see file for log format) Note that for very small or blatantly obvious problems (such as ...
Near line 68: Unpaired symbol: ‘]’ seems to be missing
Context: ...ntributing? How about checking out some [good first issues](https://github.com/pr...
Near line 69: Consider adding a comma here.
Context: ...t checking out some good first issues - Please make sure to runmake format
before e...
Near line 71: Consider using either the past participle “had” or the present participle “having” here.
Context: ...ommit - the easiest way to do this is have your editor run it for you upon saving ...
Near line 71: A comma may be missing after the conjunctive/linking adverb ‘Additionally’.
Context: ...itor run it for you upon saving a file. Additionally please ensure that your code is lint ...
Near line 78: Consider adding a comma after ‘Ideally’ for more clarity.
Context: ...t that PRs are categorically broken up. Ideally each PR addresses only a single issue. ...
Near line 80: The plural noun “PRs” cannot be used with the article “a”. Did you mean “a separate PR” or “separate PRs”?
Context: ...ring and cleanup should be submitted as a separate PRs from bugfixes/feature-additions. Draft...
Near line 91: Three successive sentences begin with the same word. Consider rewording the sentence or use a thesaurus to find a synonym.
Context: ... approved by two or more maintainers. - It must must pass all required Github acti...
Near line 91: Possible typo: you repeated a word
Context: ...proved by two or more maintainers. - It must must pass all required Github action checks....
Near line 103: It seems that a comma is missing.
Context: ...ss for reviewing PRs When reviewing PRs please use the following review explanations: ...
Near line 114: ‘surface level’ might be wordy. Consider a shorter alternative.
Context: ... PR comments. - If you are only making "surface level" reviews, submit any notes as `Comments...
Near line 159: Three successive sentences begin with the same word. Consider rewording the sentence or use a thesaurus to find a synonym.
Context: ...u can runmake proto-format
command. For linting and checking breaking changes, ...
Near line 185: The phrase “a variety of” may be wordy. To make your writing clearer, consider replacing it.
Context: ...therwise. When testing a function under a variety of different inputs, we prefer to use [tab...
Near line 186: The adjective “table-driven” is spelled with a hyphen.
Context: ... of different inputs, we prefer to use [table driven tests](https://github.com/golang/go/wik...
Near line 189: Did you mean “its” (the possessive pronoun)?
Context: ...tests. Each test case should be run as it's own sub-test, e.g. using
t.Run`. Mess...
Near line 189: This word is normally spelled as one.
Context: ...ach test case should be run as it's own sub-test, e.g. usingt.Run
. Messages should b...
Near line 222: Three successive sentences begin with the same word. Consider rewording the sentence or use a thesaurus to find a synonym.
Context: ...d branches must not failmake lint
. - Protected branches should not fail any Github act...
Near line 260: Consider replacing ‘only’ with a different word to let your writing stand out.
Context: ...ration is "testnet
". Git tags should only be used for releases. A release is auto...
Near line 264: ‘Prior to’ might be wordy. Consider a shorter alternative.
Context: ...xbranches. E.g. on
release/v1.13.x. Prior to
v1.13.0`, release tags were created on...
Near line 285: ‘off of’ might be wordy. Consider a shorter alternative.
Context: ...preformed. Oncemainnet
has upgraded off of a version, that version is no longer su...
Near line 293: This sentence should probably be started with a verb instead of the noun ‘PR’. If not, consider inserting a comma for better clarity.
Context: ...anch. 3. Create the new version tag. 4. PR the.x
branch back tomain
. #### 1...
Near line 297: Consider inserting a comma after ‘yet’ to make the sentence clearer.
Context: ...ch if Needed If a.x
branch does not yet exist for the desired minor version, on...
Near line 330: Two consecutive dots
Context: ......diff is preferred over the two dot
..` one for these links. For release ca...
Near line 349: This sentence should probably be started with a verb instead of the noun ‘Checkout’. If not, consider inserting a comma for better clarity.
Context: ...to-date repo info. E.g.git fetch
. 3. Checkout the.x
branch and make sure it's up-t...
Near line 356: This sentence should probably be started with a verb instead of the noun ‘PR’. If not, consider inserting a comma for better clarity.
Context: ...ew release page to be created. #### 4. PR the .x Branch Back to Main This PR sho...SECURITY.md (4)
Near line 21: As an alternative to the over-used intensifier ‘very’, consider replacing this phrase.
Context: ...l details of the vulnerabilities. It is very important to let us know how we can reproduce you...
Near line 25: Consider using “email”.
Context: ...blic) issue is security-related, please send an email to **[[email protected]](mailto:secur...
Near line 27: Consider using the synonym “brief” (= concise, using a few words, not lasting long) to strengthen your wording.
Context: ...email should include the issue ID and a short description of why it should be handled...
Near line 57: It seems that a comma is missing.
Context: ...ns on how this process could be improved please submit a pull request or email **[secur...
Additional comments not posted (58)
internal/handlers/aggregate_events_test.go (2)
7-7
: Updated import tocometbft
aligns with the project's transition from Tendermint to CometBFT.
Line range hint
10-36
: Ensure comprehensive test coverage for the new event aggregation logic under different transaction scenarios.cmd/provenanced/config/filenames.go (2)
20-21
: Updated filename for cometbft configuration reflects the transition from Tendermint to CometBFT.
48-50
: FunctionGetFullPathToCmtConf
correctly constructs the path to the cometbft configuration file.cmd/provenanced/cmd/testnet_test.go (2)
19-19
: Updated import tocosmos-sdk/types
is consistent with the usage in the test.
30-36
: Disabling address caching for tests involving different HRP conversions is a prudent measure to avoid flaky tests.cmd/provenanced/config/interceptor.go (2)
28-28
: The functionInterceptConfigsPreRunHandler
is updated to handle configurations for CometBFT, reflecting the project's transition.
46-57
: Setting a different default keyring backend based on the testnet flag before loading config files is a good practice to ensure environment-specific configurations.cmd/provenanced/cmd/pre_upgrade.go (2)
75-75
: FunctionExtractCmtConfig
correctly extracts the CometBFT configuration, aligning with the project's transition.
84-86
: Automatically updating thebroadcast_mode
to "sync" if it is set to "block" is a necessary adjustment given the deprecation of the "block" mode.cmd/provenanced/config/client.go (2)
25-30
: Updated default values forClientConfig
reflect the new standards for chain ID, keyring backend, output, node, and broadcast mode.
84-84
: Validation forbroadcast_mode
correctly includes "sync" and "async", aligning with the updated configurations..golangci.yml (1)
Line range hint
1-1
: Configuration to disable running tests during linting can help focus on static analysis during development.SECURITY.md (2)
16-16
: Including encryption instructions for critical security reports is a good practice to ensure sensitive information is securely communicated.
30-30
: Providing a link to report issues related to the Cosmos SDK or CometBFT not found in this repo helps in directing reports to the appropriate channels.cmd/provenanced/cmd/init.go (2)
Line range hint
38-52
: LGTM! The command setup and flag configurations are correctly implemented.
Line range hint
54-158
: Ensure the removal ofConsensus.TimeoutCommit
handling is reflected across the codebase.cmd/provenanced/cmd/pre_upgrade_test.go (5)
101-103
: LGTM! The environment setup and command execution are correctly handled.
Line range hint
193-208
: LGTM! The dummy command setup and context configuration are correctly implemented.
Line range hint
222-231
: LGTM! The home directory creation and configuration saving are correctly implemented.
235-236
: LGTM! The extension of home directory creation to include configuration packing is correctly implemented.
263-263
: Verify the handling of the broadcast mode change in the pre-upgrade command.Verification successful
The handling of the broadcast mode change in the pre-upgrade command has been verified. Both the implementation and the test case reflect the update from "block" to "sync", which is consistent with the new requirements. The message in the test case accurately represents this change.
cmd/provenanced/cmd/pre_upgrade.go
: Implementation updates thebroadcast_mode
and logs the change.cmd/provenanced/cmd/pre_upgrade_test.go
: Test case includes a message verifying the update.Scripts Executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify that the broadcast mode change handling in the pre-upgrade command is consistent with the new requirements. # Test: Search for the handling of the broadcast mode change. Expect: Correct handling according to the new requirements. rg --type go 'broadcast_mode'Length of output: 369
cmd/provenanced/config/manager_test.go (5)
54-54
: LGTM! The dummy command setup and context configuration are correctly implemented.
128-130
: LGTM! The test for writing and reading index events is correctly implemented.
148-150
: LGTM! The test for loading default configurations from a packed config is correctly implemented.
206-206
: Verify the handling of invalid packed files in the configuration management.Verification successful
The review of the handling of invalid packed files in the configuration management has been completed based on the provided script output. The results confirm that the system is equipped with appropriate error handling for invalid packed configurations. Specific tests in
manager_test.go
validate this by checking for expected error messages when invalid data is introduced. This aligns with the expected behavior for robust configuration management.
cmd/provenanced/config/manager_test.go
: Contains tests that simulate invalid packed configurations and verify that the system responds with the correct error messages.Scripts Executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify that the handling of invalid packed files in the configuration management is consistent with the expected behavior. # Test: Search for the handling of invalid packed files. Expect: Correct error handling according to the expected behavior. rg --type go 'packed config'Length of output: 4457
378-378
: LGTM! The test for default CometBFT configuration settings is correctly implemented.cmd/provenanced/config/reflector_test.go (2)
42-45
: New structSubThing3
added.This addition seems to be well-defined and follows the existing pattern of struct definitions in this file.
53-54
: UpdatedMainThing
struct to includeSubThing3
.The integration of
SubThing3
intoMainThing
is correctly implemented and aligns with the structural changes made in this file.Makefile (1)
17-17
: Updated to fetch CometBFT version.The
CMTVERSION
variable is correctly set to fetch the version of CometBFT using Go's package management, which aligns with the project's transition from Tendermint to CometBFT.CONTRIBUTING.md (1)
Line range hint
185-232
: Ensure the logic inFindEntries
method handles edge cases correctly, especially with keys that start or end with a period.Verification successful
The verification of the
FindEntries
method's logic for handling keys with periods has been successfully completed. The method is designed to handle various scenarios involving keys with periods, including exact matches, prefixes, and suffixes. The test cases inreflector_test.go
comprehensively cover these scenarios, ensuring that the method behaves as expected under different conditions.
- The method checks for exact matches first. If an exact match is found, it returns immediately without further searches.
- If no exact match is found, it looks for keys that start with the specified key followed by a period, and keys that end with a period followed by the specified key.
- Comprehensive test cases confirm the method's correct behavior in handling keys with periods, including scenarios where keys start or end with a period.
This thorough testing and implementation confirm that the method handles edge cases correctly, as required by the review comment.
Scripts Executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify the logic in `FindEntries` method for handling keys with periods. # Test: Search for usage of `FindEntries`. Expect: Proper handling of edge cases. rg --type go $'FindEntries'Length of output: 1469
Script:
#!/bin/bash # Description: Verify the implementation and test cases for `FindEntries` method. # Fetch the implementation of `FindEntries` method cat cmd/provenanced/config/reflector.go # Fetch the test cases for `FindEntries` method cat cmd/provenanced/config/reflector_test.goLength of output: 36971
cmd/provenanced/config/reflector.go (1)
143-178
: Verify the new logic inFindEntries
for handling different search patterns and ensure it handles edge cases correctly.Verification successful
The
FindEntries
function inreflector.go
has been thoroughly tested to handle various search patterns and edge cases. The test suite inreflector_test.go
includes multiple scenarios that validate the function's ability to correctly identify exact matches, handle nested structures, and differentiate between similar keys in different contexts. This comprehensive testing ensures that the function behaves as expected under a wide range of conditions.
- Test Coverage: The function is tested for exact matches, nested section searches, and handling of keys without direct matches but within the same section.
- Edge Case Handling: Tests such as "exact entry unique base" and "a section without period" confirm that the function can handle both straightforward and complex search scenarios effectively.
Given this information, the new logic in
FindEntries
for handling different search patterns appears to be robust and well-implemented.Scripts Executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify the new logic in `FindEntries` for handling different search patterns. # Test: Search for usage of `FindEntries`. Expect: Proper handling of edge cases. rg --type go $'FindEntries'Length of output: 1469
Script:
#!/bin/bash # Description: Extract test cases related to `FindEntries` from `reflector_test.go` to verify edge case handling. # Extract the relevant test cases rg --type go --context 10 "TestFieldValueMap_FindEntries" cmd/provenanced/config/reflector_test.goLength of output: 632
Script:
#!/bin/bash # Description: Extract complete test cases related to `FindEntries` from `reflector_test.go` to verify edge case handling. # Extract the complete test cases including assertions rg --type go --context 30 "TestFieldValueMap_FindEntries" cmd/provenanced/config/reflector_test.goLength of output: 1816
Script:
#!/bin/bash # Description: Extract further test cases related to `FindEntries` from `reflector_test.go` to verify edge case handling comprehensively. # Extract further lines of the test cases including assertions rg --type go --context 50 "TestFieldValueMap_FindEntries" cmd/provenanced/config/reflector_test.goLength of output: 3093
cmd/provenanced/config/manager.go (1)
38-40
: Ensure proper error handling for CometBFT configuration extraction.Verification successful
The error handling for the CometBFT configuration extraction function
ExtractCmtConfig
and its variantExtractCmtConfigAndMap
appears to be robust and consistently implemented across the codebase. Each usage of these functions properly checks and handles errors, either by returning the error directly or by using assertions in test scenarios. This includes the specific implementation incmd/provenanced/config/manager.go
where the error is formatted and returned, providing clear context on the nature of the issue.
cmd/provenanced/cmd/config.go
cmd/provenanced/cmd/pre_upgrade_test.go
cmd/provenanced/cmd/pre_upgrade.go
cmd/provenanced/cmd/init.go
cmd/provenanced/cmd/config_test.go
cmd/provenanced/config/manager_test.go
cmd/provenanced/config/manager.go
Scripts Executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify that the error handling for CometBFT configuration extraction is robust and covers all potential failure modes. # Test: Search for the function usage. Expect: Proper error handling in all usages. rg --type go $'ExtractCmtConfig'Length of output: 1973
cmd/provenanced/cmd/config.go (9)
64-65
: Renaming "tendermint" to "cometbft" in configuration commands aligns with the project's transition from Tendermint to CometBFT.
74-78
: Ensure that the example usage in comments is updated to reflect the new configuration commands.Verification successful
The example usage in the comments of
cmd/provenanced/cmd/config.go
has been updated to reflect the new configuration commands as evidenced by the presence of commands likeget api
,get app
, andget cmt
in the file. This aligns with the expectations set in the review comment.
cmd/provenanced/cmd/config.go
: Contains updated example usage withget api
,get app
, andget cmt
.Scripts Executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify that the example usage in comments reflects the new configuration commands. # Test: Search for outdated command usage. Expect: No occurrences of old commands. rg --type go $'get tendermint'Length of output: 31
Script:
#!/bin/bash # Description: Further verify that the example usage in comments reflects the new configuration commands. # Test: Search for the presence of new configuration commands to ensure they are being used in the example usage. rg --type go $'get api|get app|get cmt'Length of output: 956
150-151
: The update in theConfigChangedCmd
function to use "cometbft" instead of "tendermint" is consistent with the project's framework transition.
247-249
: Proper error handling inrunConfigGetCmd
ensures robustness when fetching CometBFT configuration fields.
272-276
: The deprecation warning for "tendermint" and "tm" is clear and instructive, guiding users towards the new terminology.
362-364
: Error handling inrunConfigSetCmd
when fetching CometBFT configuration fields is crucial for maintaining system stability.
473-475
: Proper error handling inrunConfigChangedCmd
when fetching CometBFT configuration fields is essential for system reliability.
503-505
: The deprecation warning for "tendermint" and "tm" inrunConfigChangedCmd
is consistent and helps in transitioning users to the new terminology.
688-692
: The creation of a section header for CometBFT configuration inmakeCmtConfigHeader
function is well-implemented, reflecting the new system architecture.cmd/provenanced/cmd/config_test.go (16)
43-47
: Renaming of header and base filename strings to align with CometBFT terminology.
56-56
: Global default forKeyringBackend
set to "os" aligns with the PR's objectives to standardize configurations across environments.
63-63
: Initialization ofserverCtx
withDefaultCmtConfig
reflects the updated default configurations for CometBFT.
74-78
: Updated header and base filenames in the test setup to reflect the new CometBFT configuration files.
108-115
: The methodExtractCmtConfig
is correctly used to extract CometBFT configurations, andSaveConfigs
is adapted to handle the new configuration structure.
141-142
: Method to generate header lines for CometBFT configuration files correctly reflects the new file structure.
157-158
: Method to generate updated lines for CometBFT configuration files correctly reflects changes in configuration values.
173-174
: Method to generate diff header lines for CometBFT configuration files correctly reflects the comparison against default values.
181-184
: Deprecation warning for "tendermint" and "tm" options is clear and guides users towards the new "cometbft", "comet", or "cmt" options.
201-212
: MethodsetDefaultKeyringBackend
provides a clean way to temporarily change and restore theDefaultKeyringBackend
setting, useful for testing different configurations.
764-766
: Validation error message for settinglog_format
in CometBFT configuration is clear and specifies the acceptable values.
793-793
: Use of regular expressions to verify the output of configuration updates is a robust method to ensure that changes are reflected in the output.
839-839
: Test case for updating thefilter_peers
setting in CometBFT configuration is well-defined and checks for the correct update signal in the output.
936-939
: Test cases for setting multiple CometBFT configuration entries in one command are comprehensive and ensure that changes are correctly applied.
1087-1087
: The method for writing a custom CometBFT configuration file for testing is effective in creating a controlled environment for testing configuration management.
1141-1144
: Test to ensure that thedb_backend
value in CometBFT configuration remains unchanged after updates is crucial for verifying the stability of critical settings.
…oniker that gets chosen is different on different computers.
…names are unique accross the multiple configs.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
Review Details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (2)
- cmd/provenanced/cmd/config_test.go (26 hunks)
- cmd/provenanced/config/manager_test.go (15 hunks)
Files not summarized due to errors (1)
- cmd/provenanced/cmd/config_test.go: Error: Message exceeds token limit
Files skipped from review as they are similar to previous changes (1)
- cmd/provenanced/config/manager_test.go
Additional comments not posted (19)
cmd/provenanced/cmd/config_test.go (19)
43-47
: Update variable names to align with CometBFT terminology.
56-56
: Ensure that the default keyring backend is set to "os" for all environments except testnet.Verification successful
The verification process confirms that the default keyring backend is set to "os" for all environments except testnet, which aligns with the requirements specified in the review comment. Here are the key findings:
- The default setting is "os" in general configurations as seen in
client.go
.- For testnet, the backend can be configured differently as indicated in
testnet.go
andinterceptor.go
.This setup ensures that the default configuration is secure and consistent, while still allowing flexibility for test environments.
Scripts Executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify that the default keyring backend is set to "os" for all environments except testnet. # Test: Search for the keyring backend setting across configurations. Expect: "os" for all except testnet. rg --type go $'DefaultKeyringBackend'Length of output: 1902
63-63
: Initialization ofserverCtx
withDefaultCmtConfig()
aligns with the new CometBFT framework.
74-78
: Correctly updated header and base file names to reflect CometBFT terminology.
108-115
: Proper extraction and saving of configuration settings for CometBFT.
141-142
: Ensure the header line for CometBFT configuration is correctly formatted.
157-158
: Ensure the update line for CometBFT configuration is correctly formatted.
173-174
: Ensure the diff header line for CometBFT configuration is correctly formatted.
181-184
: Deprecation message for "tendermint" and "tm" options is clear and guides the user towards new options.
201-212
: The methodsetDefaultKeyringBackend
correctly handles the temporary change of theDefaultKeyringBackend
setting.
248-413
: Comprehensive testing of theget
command with various configurations and expected outputs.
Line range hint
452-557
: Tests for fetching specific configuration keys are well-structured and cover multiple scenarios.
Line range hint
634-733
: Tests for thechanged
command are comprehensive and verify that the output matches expected values.
742-755
: Test for changing the default keyring backend and verifying the output.
770-772
: Validation for theset
command correctly handles invalid values for CometBFT configurations.
Line range hint
799-900
: Tests for theset
command verify that updates to configuration settings are reflected in the output.
Line range hint
942-973
: Multi-entry set operations are correctly handled and tested.
1000-1042
: Tests for packing and unpacking configurations verify the integrity and correctness of the operations.
Line range hint
1093-1147
: Tests for updating configurations verify that changes are applied correctly and existing values are preserved where expected.
…ould now refer to comet.
…ies instead of raw string slicing.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 2
Out of diff range and nitpick comments (1)
cmd/provenanced/cmd/pre_upgrade_test.go (1)
Line range hint
193-208
: Refactor to use the updated CometBFT configuration in themakeDummyCmd
function.- serverCtx := server.NewContext(clientCtx.Viper, config.DefaultCmtConfig(), log.NewNopLogger()) + serverCtx := server.NewContext(clientCtx.Viper, config.DefaultCometBFTConfig(), log.NewNopLogger())
Review Details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (4)
- cmd/provenanced/cmd/pre_upgrade_test.go (11 hunks)
- cmd/provenanced/config/manager.go (18 hunks)
- cmd/provenanced/config/manager_test.go (15 hunks)
- internal/handlers/msg_service_router_test.go (5 hunks)
Files skipped from review as they are similar to previous changes (2)
- cmd/provenanced/config/manager.go
- cmd/provenanced/config/manager_test.go
Additional comments not posted (12)
cmd/provenanced/cmd/pre_upgrade_test.go (3)
Line range hint
222-231
: Ensure that the directory creation and configuration saving are handled correctly in thenewHome
function.
235-236
: Verify the configuration packing logic in thenewHomePacked
function.Verification successful
The implementation and usage of the
PackConfig
function across the codebase have been verified. The function is correctly implemented incmd/provenanced/config/manager.go
and is used with appropriate error handling incmd/provenanced/cmd/pre_upgrade_test.go
and other test files. This confirms that the configuration packing logic is implemented as expected and handles errors properly.
- Implementation Location:
cmd/provenanced/config/manager.go
- Usage in Tests:
cmd/provenanced/cmd/pre_upgrade_test.go
,cmd/provenanced/config/manager_test.go
Scripts Executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify the configuration packing logic. # Test: Search for the function usage. Expect: Correct implementation and error handling. rg --type go $'config.PackConfig'Length of output: 210
Script:
#!/bin/bash # Description: Inspect the implementation of provconfig.PackConfig to verify the packing logic. # Test: Search for the function implementation and error handling. ast-grep --lang go --pattern $'func PackConfig($_) { $$$ }'Length of output: 62
Script:
#!/bin/bash # Description: Broaden the search to locate the PackConfig function or any aliases. # Test: Use ripgrep to search for any reference to PackConfig in the Go files. rg --type go 'PackConfig'Length of output: 696
465-468
: Ensure the CometBFT configuration extraction and root setting are correctly implemented.internal/handlers/msg_service_router_test.go (9)
Line range hint
91-109
: The test for registering message services is well-implemented and covers the expected behaviors correctly.
Line range hint
111-131
: The test for registering a message service twice is correctly implemented, ensuring that the application panics as expected on the second registration attempt.
Line range hint
133-235
: The test for handling failed transactions due to insufficient funds is comprehensive and correctly verifies the expected outcomes and state changes.
Line range hint
237-387
: The tests for message services handling different fee scenarios are well-implemented, covering a variety of cases and correctly verifying the outcomes.
Line range hint
389-451
: The test for message fees with a specified recipient is correctly implemented, ensuring that fees are appropriately distributed and verifying the outcomes through account balances and events.
Line range hint
453-593
: The tests for authorization features in message services are well-implemented, covering the setup and use of authz grants and correctly verifying the transaction outcomes.
Line range hint
595-737
: The tests for assessing custom message fees are comprehensive and correctly implemented, verifying the fee distribution and outcomes through detailed checks.
Line range hint
739-881
: The tests for assessing custom message fees with specified bips are well-implemented, ensuring that fees are appropriately split and verifying the outcomes through detailed checks.
Line range hint
883-1025
: The test for assessing custom message fees without a specified recipient is correctly implemented, ensuring that the full fee is directed to the module account and verifying the outcomes.
# Conflicts: # .github/workflows/test.yml
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
Out of diff range and nitpick comments (4)
CHANGELOG.md (4)
Line range hint
87-332
: Consistent list marker style should be used.- - + *
Line range hint
206-881
: Bare URLs should be linked to text for better readability and to avoid direct exposure of links.- https://github.com/provenance-io/provenance/issues/105 + [Issue #105](https://github.com/provenance-io/provenance/issues/105)
Line range hint
923-923
: Remove unnecessary spaces inside emphasis markers.- * `MsgAddContractSpecToScopeSpecRequest`and `MsgDeleteContractSpecFromScopeSpecRequest` + * `MsgAddContractSpecToScopeSpecRequest` and `MsgDeleteContractSpecFromScopeSpecRequest`
Line range hint
319-319
: Remove unnecessary spaces inside code span elements.- ` MsgAddContractSpecToScopeSpecRequest ` + `MsgAddContractSpecToScopeSpecRequest`
Review Details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (2)
- .github/workflows/test.yml (2 hunks)
- CHANGELOG.md (1 hunks)
Files skipped from review as they are similar to previous changes (1)
- .github/workflows/test.yml
# Conflicts: # .github/workflows/test.yml # CHANGELOG.md
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
Out of diff range and nitpick comments (2)
CHANGELOG.md (2)
Line range hint
210-631
: Convert bare URLs to markdown links for better readability.- https://github.com/provenance-io/provenance/issues/210 + [Issue #210](https://github.com/provenance-io/provenance/issues/210)Please update all bare URLs throughout the document to use markdown link syntax.
Line range hint
799-1373
: Remove extra blank lines.-
Please ensure that there is at most one blank line separating sections for clarity and consistency in markdown files.
Review Details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (2)
- .github/workflows/test.yml (1 hunks)
- CHANGELOG.md (1 hunks)
Files skipped from review as they are similar to previous changes (1)
- .github/workflows/test.yml
Additional comments not posted (1)
CHANGELOG.md (1)
Line range hint
91-336
: Consistent use of list markers is recommended.- * message #<issue-number> + * message #<issue-number>Ensure all list items in markdown use the same style for bullets, preferably asterisks (*) instead of dashes (-) for consistency.
Skipped due to learnings
User: SpicyLemon URL: https://github.com/provenance-io/provenance/pull/1930 Timestamp: 2024-04-16T20:13:24.272Z Learning: Avoid pointing out list item marker inconsistencies in the CHANGELOG.md or similar files as per user request.
…use it was asking for an address previously encoded with the wrong hrp.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 2
Out of diff range and nitpick comments (17)
CHANGELOG.md (17)
Line range hint
92-106
: Ensure consistency in list formatting.Use asterisks (*) instead of dashes (-) for unordered lists to maintain consistency with the rest of the document.
- Bump `github.com/cosmos/ibc-go/v8` from 8.0.0 to 8.2.1 ([#1910](https://github.com/provenance-io/provenance/pull/1910), [#1956](https://github.com/provenance-io/provenance/pull/1956)) - Bump `google.golang.org/grpc` from 1.62.1 to 1.63.2 ([#1903](https://github.com/provenance-io/provenance/pull/1903), [#1918](https://github.com/provenance-io/provenance/pull/1918)) - Bump `bufbuild/buf-breaking-action` from 1.1.3 to 1.1.4 ([#1894](https://github.com/provenance-io/provenance/pull/1894)) - Bump `bufbuild/buf-lint-action` from 1.1.0 to 1.1.1 ([#1895](https://github.com/provenance-io/provenance/pull/1895)) - Bump `bufbuild/buf-setup-action` from 1.30.0 to 1.31.0 ([#1904](https://github.com/provenance-io/provenance/pull/1904), [#1949](https://github.com/provenance-io/provenance/pull/1949)) - Bump `github.com/cometbft/cometbft` from 0.38.5 to 0.38.7 ([#1912](https://github.com/provenance-io/provenance/pull/1912), [#1959](https://github.com/provenance-io/provenance/pull/1959)) - Bump `cosmossdk.io/x/upgrade` from 0.1.0 to 0.1.1 ([#1913](https://github.com/provenance-io/provenance/pull/1913)) - Bump `github.com/hashicorp/go-metrics` from 0.5.2 to 0.5.3 ([#1914](https://github.com/provenance-io/provenance/pull/1914)) - Bump `peter-evans/create-pull-request` from 6.0.2 to 6.0.5 ([#1929](https://github.com/provenance-io/provenance/pull/1929), [#1940](https://github.com/provenance-io/provenance/pull/1940), [#1955](https://github.com/provenance-io/provenance/pull/1955)) - Bump `cosmossdk.io/x/tx` from 0.13.1 to 0.13.3 ([#1928](https://github.com/provenance-io/provenance/pull/1928), [#1944](https://github.com/provenance-io/provenance/pull/1944)) - Bump `cosmwasm-std` from 1.4.1 to 1.4.4 ([#1950](https://github.com/provenance-io/provenance/pull/1950)) - Bump `golangci/golangci-lint-action` from 4 to 6 ([#1951](https://github.com/provenance-io/provenance/pull/1951), [#1965](https://github.com/provenance-io/provenance/pull/1965)) - Bump `google.golang.org/protobuf` from 1.33.0 to 1.34.1 ([#1960](https://github.com/provenance-io/provenance/pull/1960), [#1966](https://github.com/provenance-io/provenance/pull/1966)) - Bump `github.com/hashicorp/go-getter` from 1.7.3 to 1.7.4 ([#1958](https://github.com/provenance-io/provenance/pull/1958)) - Bump `golang.org/x/text` from 0.14.0 to 0.15.0 ([#1964](https://github.com/provenance-io/provenance/pull/1964)) + * Bump `github.com/cosmos/ibc-go/v8` from 8.0.0 to 8.2.1 ([#1910](https://github.com/provenance-io/provenance/pull/1910), [#1956](https://github.com/provenance-io/provenance/pull/1956)) + * Bump `google.golang.org/grpc` from 1.62.1 to 1.63.2 ([#1903](https://github.com/provenance-io/provenance/pull/1903), [#1918](https://github.com/provenance-io/provenance/pull/1918)) + * Bump `bufbuild/buf-breaking-action` from 1.1.3 to 1.1.4 ([#1894](https://github.com/provenance-io/provenance/pull/1894)) + * Bump `bufbuild/buf-lint-action` from 1.1.0 to 1.1.1 ([#1895](https://github.com/provenance-io/provenance/pull/1895)) + * Bump `bufbuild/buf-setup-action` from 1.30.0 to 1.31.0 ([#1904](https://github.com/provenance-io/provenance/pull/1904), [#1949](https://github.com/provenance-io/provenance/pull/1949)) + * Bump `github.com/cometbft/cometbft` from 0.38.5 to 0.38.7 ([#1912](https://github.com/provenance-io/provenance/pull/1912), [#1959](https://github.com/provenance-io/provenance/pull/1959)) + * Bump `cosmossdk.io/x/upgrade` from 0.1.0 to 0.1.1 ([#1913](https://github.com/provenance-io/provenance/pull/1913)) + * Bump `github.com/hashicorp/go-metrics` from 0.5.2 to 0.5.3 ([#1914](https://github.com/provenance-io/provenance/pull/1914)) + * Bump `peter-evans/create-pull-request` from 6.0.2 to 6.0.5 ([#1929](https://github.com/provenance-io/provenance/pull/1929), [#1940](https://github.com/provenance-io/provenance/pull/1940), [#1955](https://github.com/provenance-io/provenance/pull/1955)) + * Bump `cosmossdk.io/x/tx` from 0.13.1 to 0.13.3 ([#1928](https://github.com/provenance-io/provenance/pull/1928), [#1944](https://github.com/provenance-io/provenance/pull/1944)) + * Bump `cosmwasm-std` from 1.4.1 to 1.4.4 ([#1950](https://github.com/provenance-io/provenance/pull/1950)) + * Bump `golangci/golangci-lint-action` from 4 to 6 ([#1951](https://github.com/provenance-io/provenance/pull/1951), [#1965](https://github.com/provenance-io/provenance/pull/1965)) + * Bump `google.golang.org/protobuf` from 1.33.0 to 1.34.1 ([#1960](https://github.com/provenance-io/provenance/pull/1960), [#1966](https://github.com/provenance-io/provenance/pull/1966)) + * Bump `github.com/hashicorp/go-getter` from 1.7.3 to 1.7.4 ([#1958](https://github.com/provenance-io/provenance/pull/1958)) + * Bump `golang.org/x/text` from 0.14.0 to 0.15.0 ([#1964](https://github.com/provenance-io/provenance/pull/1964))
Line range hint
211-211
: Avoid using bare URLs.Use markdown syntax for links to improve readability.
- https://keepachangelog.com/en/1.0.0/ + [https://keepachangelog.com/en/1.0.0/](https://keepachangelog.com/en/1.0.0/)
Line range hint
242-242
: Avoid using bare URLs.Use markdown syntax for links to improve readability.
- [#1772](https://github.com/provenance-io/provenance/issues/1772). + [#1772](https://github.com/provenance-io/provenance/issues/1772)
Line range hint
341-341
: Avoid using bare URLs.Use markdown syntax for links to improve readability.
- [#1760](https://github.com/provenance-io/provenance/issues/1760). + [#1760](https://github.com/provenance-io/provenance/issues/1760)
Line range hint
391-391
: Avoid using bare URLs.Use markdown syntax for links to improve readability.
- [#1760](https://github.com/provenance-io/provenance/issues/1760). + [#1760](https://github.com/provenance-io/provenance/issues/1760)
Line range hint
403-403
: Avoid using bare URLs.Use markdown syntax for links to improve readability.
- [#1760](https://github.com/provenance-io/provenance/issues/1760). + [#1760](https://github.com/provenance-io/provenance/issues/1760)
Line range hint
419-419
: Avoid using bare URLs.Use markdown syntax for links to improve readability.
- [#1760](https://github.com/provenance-io/provenance/issues/1760). + [#1760](https://github.com/provenance-io/provenance/issues/1760)
Line range hint
490-490
: Avoid using bare URLs.Use markdown syntax for links to improve readability.
- [#1760](https://github.com/provenance-io/provenance/issues/1760). + [#1760](https://github.com/provenance-io/provenance/issues/1760)
Line range hint
501-501
: Avoid using bare URLs.Use markdown syntax for links to improve readability.
- [#1760](https://github.com/provenance-io/provenance/issues/1760). + [#1760](https://github.com/provenance-io/provenance/issues/1760)
Line range hint
509-509
: Avoid using bare URLs.Use markdown syntax for links to improve readability.
- [#1760](https://github.com/provenance-io/provenance/issues/1760). + [#1760](https://github.com/provenance-io/provenance/issues/1760)
Line range hint
545-545
: Avoid using bare URLs.Use markdown syntax for links to improve readability.
- [#1760](https://github.com/provenance-io/provenance/issues/1760). + [#1760](https://github.com/provenance-io/provenance/issues/1760)
Line range hint
570-570
: Avoid using bare URLs.Use markdown syntax for links to improve readability.
- [#1760](https://github.com/provenance-io/provenance/issues/1760). + [#1760](https://github.com/provenance-io/provenance/issues/1760)
Line range hint
582-582
: Avoid using bare URLs.Use markdown syntax for links to improve readability.
- [#1760](https://github.com/provenance-io/provenance/issues/1760). + [#1760](https://github.com/provenance-io/provenance/issues/1760)
Line range hint
628-632
: Avoid using bare URLs.Use markdown syntax for links to improve readability.
- [#1760](https://github.com/provenance-io/provenance/issues/1760). + [#1760](https://github.com/provenance-io/provenance/issues/1760)
Line range hint
679-679
: Avoid using bare URLs.Use markdown syntax for links to improve readability.
- [#1760](https://github.com/provenance-io/provenance/issues/1760). + [#1760](https://github.com/provenance-io/provenance/issues/1760)
Line range hint
720-720
: Avoid using bare URLs.Use markdown syntax for links to improve readability.
- [#1760](https://github.com/provenance-io/provenance/issues/1760). + [#1760](https://github.com/provenance-io/provenance/issues/1760)
Line range hint
773-773
: Avoid using bare URLs.Use markdown syntax for links to improve readability.
- [#1760](https://github.com/provenance-io/provenance/issues/1760). + [#1760](https://github.com/provenance-io/provenance/issues/1760)
Review Details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (2)
- CHANGELOG.md (1 hunks)
- cmd/provenanced/cmd/genaccounts_test.go (2 hunks)
Files not reviewed due to errors (1)
- cmd/provenanced/cmd/genaccounts_test.go (no review received)
* [1760]: Turn off address caching in Test_TestnetCmd because it's caching addresses with the wrong hrp for other tests. This doesn't show up if running tests one file at a time, but causes panics when all the tests in that package are run together. * [1760]: Fix some config unit tests that started failing because they were using fields that no longer exist. * [1760]: Update the config get and changed sub-commands to check all the configs for entries, possibly outputting multiple. * [1760]: Start renaming all the tendermint and tm stuff in the config command. Deprecate 'tendermint' and 'tm' as options for 'get' and 'changed', and add 'cometbft', 'comet', and 'cmt' as options. * [1760]: Set the correct TMCoreSemVer variable when building. * [1760]: Take the tendermint libraries off the allowed import list. Update CONTRIBUTING and SECURITY for the change from tendermint to cometbft. * [1760]: Fix the rest of the tendermint references. * [1760]: Tweak the config get tests to check the current output against a previous value in order to identify fields that were added, removed, renamed, or have new default values. * [1760]: Update pre-upgrade command to change the broadcast mode from block to sync. Make the client config defaults public variables (instead of private consts) so we can update them dynamically if needed. * [1760]: Change the default keyring backend to os, but keep it as test for testnets. Add/Update unit tests for the broadcast mode and keyring backend changes, as well as for the pre-upgrade command changes." * [1760]: Move the setting of the default keyring backend to inside the interceptor so that it can get used when loading the files. * [1760]: Fix a gofmt thing in internal/handlers/aggregate_events_test.go. * [1760]: Add a comment to Test_TestnetCmd explaining why address caching is disabled. * [1760]: Make the config get and changed commands also look for sub-field matches and loose matches. * [1760]: Fix a couple unit tests that, when run together, were ending up with the wrong default keyring backend. * [1760]: Remove cmd/provenanced/cmd from the expected failures in the test github action. * [1760]: Add changelog entries.
Description
This PR does the following:
cmd/provenanced/cmd
tests to required passing in the github workflow.Consensus.TimeoutCommit
value from the pre-upgrade command.TMCoreSemVer
variable when building.Related to:
Here's a detailed list of config changes.
The
app.toml
file has the following changes:query-gas-limit
.iavl-lazy-loading
.telemetry.metrics-sink
.telemetry.statsd-addr
.telemetry.datadog-hostname
.api.address
to "localhost:1317" (from "0.0.0.0:1317").grpc.address
to "localhost:9090" (from "0.0.0.0:9090").rosetta.address=":8080"
.rosetta.blockchain="app"
.rosetta.denom-to-suggest="uatom"
.rosetta.enable=false
.rosetta.enable-fee-suggestion=false
.rosetta.gas-to-suggest=200000
.rosetta.network="network"
.rosetta.offline=false
.rosetta.retries=3
.grpc-web.address
.grpc-web.enable-unsafe-cors
.mempool.max-txs
.The
config.toml
file has the following changes:version
(This is for the cometbft version).fast_sync
.p2p.upnp
.mempool.version
.mempool.type
.mempool.ttl-duration
.mempool.ttl-num-blocks
.experimental_max_gossip_connections_to_persistent_peers
.experimental_max_gossip_connections_to_non_persistent_peers
.fastsync
section toblocksync
. Only has one field:version
.The
client.toml
file has the following changes:broadcast-mode
tosync
(fromblock
).keyring-backend
toos
(fromtest
), but only if the--testnet
option is NOT provided (either as a flag orPIO_TESTNET
environment variable).There is only one field that is being renamed:
blocksync.version
(wasfastsync.version
). The pre-upgrade command does not make any attempt to migrate this value, so it will receive the default ofv0
. The field comments state that the other values are deprecated, and thatv0
is the only one that should be used. Because of that, I felt that it was okay to ignore the old value instead of migrating the value. I also felt that it was very unlikely that anyone is using some other value for that field.For the most part, all these defaults only come into play when the files are initially being created. For example, if you
provenanced init
, you will get akeyring-backend
ofos
. Later, even if you provide the--testnet
flag with a command, you'll still be usingos
for that field because that's what theclient.toml
has. However, these defaults also come into play when using a packed config. For example, if you then doprovenanced config pack
, it'll see thatkeyring-backend
has the default value, and won't be included in thepacked-conf.json
. Later, if you doprovenanced config get keyring-backend
, it'll sayos
, but if youprovenanced --testnet config get keyring-backend
, you'll gettest
. Thekeyring-backend
config field is the only one that has more than one default.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.
docs/
) or specification (x/<module>/spec/
)godoc
comments.Unreleased
section inCHANGELOG.md
Files changed
in the Github PR explorerCodecov Report
in the comment section below once CI passesSummary by CodeRabbit
New Features
Deprecations
Documentation
CONTRIBUTING.md
.SECURITY.md
.Refactor
Bug Fixes