Fix the umber upgrades. #2033
Fix the umber upgrades. #2033
Conversation
…f was originally so that we can have it inject an upgrade which now overwrites the pre-blocker (instead of begin-blocker).
…lt. Those params never got defined in testnet, and mainnet has an empty entry for it. So both were just using the default value there anyway.
…et after initially being changed to testnet.
…with some other flags so that we can use them in there as needed.
…grations all failed because the needed keys weren't registered.
…'s being done (all of the params stuff is 'legacy' now, so it didn't really describe what was happening.
… ParamKeyTable functions.
…us params migration (that I updated a few commits ago).
WalkthroughThe recent changes enhance the project's functionality by updating dependencies, refactoring various initialization and configuration functions, and enhancing test coverage. This includes upgrading specific package imports, modifying key tables, refining configuration settings, and improving migration logic. The modifications streamline address and key prefix management, improve parameter setting, and ensure backward compatibility with older configurations while supporting new versions and features. Changes
Sequence DiagramssequenceDiagram
participant User
participant App
participant PrefixConfig
participant MigrationHandler
User->>App: Start Application
App->>PrefixConfig: SetConfig(testnet, seal)
PrefixConfig->>App: Configure Addresses and Prefixes
App->>MigrationHandler: Register Upgrades and Handlers
MigrationHandler->>App: Migrate Parameters
Note right of MigrationHandler: Handles new consensus types
Poem
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 (
|
|
I forced both the The slashing modules' v3 -> v4 upgrade takes a couple minutes (on my mac), but the rest of the steps are pretty snappy. |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
Outside diff range and nitpick comments (3)
app/app.go (1)
Line range hint
1361-1411: TheinjectUpgradefunction is marked for testing purposes and should not be included in production code. Ensure that this function is used appropriately and is not part of any production deployment. It's crucial to verify that this function does not introduce any unintended side effects during the upgrade process.Consider removing or isolating this function from production branches to prevent accidental execution.
CHANGELOG.md (2)
Line range hint
129-253: Consider using asterisks (*) instead of dashes (-) for unordered lists to maintain consistency with Markdown standards.- - 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).
Line range hint
283-385: Consider using asterisks (*) instead of dashes (-) for unordered lists to maintain consistency with Markdown standards.- - 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 `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))
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (9)
- CHANGELOG.md (1 hunks)
- app/app.go (9 hunks)
- app/prefix.go (1 hunks)
- app/prefix_test/prefix_test.go (1 hunks)
- app/upgrades.go (7 hunks)
- app/upgrades_test.go (2 hunks)
- cmd/provenanced/cmd/init.go (1 hunks)
- cmd/provenanced/cmd/root.go (4 hunks)
- cmd/provenanced/config/interceptor.go (2 hunks)
Files skipped from review due to trivial changes (1)
- app/upgrades_test.go
Additional context used
Markdownlint
CHANGELOG.md
129-129: Expected: asterisk; Actual: dash (MD004, ul-style)
Unordered list style
130-130: Expected: asterisk; Actual: dash (MD004, ul-style)
Unordered list style
131-131: Expected: asterisk; Actual: dash (MD004, ul-style)
Unordered list style
132-132: Expected: asterisk; Actual: dash (MD004, ul-style)
Unordered list style
133-133: Expected: asterisk; Actual: dash (MD004, ul-style)
Unordered list style
134-134: Expected: asterisk; Actual: dash (MD004, ul-style)
Unordered list style
135-135: Expected: asterisk; Actual: dash (MD004, ul-style)
Unordered list style
136-136: Expected: asterisk; Actual: dash (MD004, ul-style)
Unordered list style
137-137: Expected: asterisk; Actual: dash (MD004, ul-style)
Unordered list style
138-138: Expected: asterisk; Actual: dash (MD004, ul-style)
Unordered list style
139-139: Expected: asterisk; Actual: dash (MD004, ul-style)
Unordered list style
140-140: Expected: asterisk; Actual: dash (MD004, ul-style)
Unordered list style
141-141: Expected: asterisk; Actual: dash (MD004, ul-style)
Unordered list style
142-142: Expected: asterisk; Actual: dash (MD004, ul-style)
Unordered list style
143-143: Expected: asterisk; Actual: dash (MD004, ul-style)
Unordered list style
144-144: Expected: asterisk; Actual: dash (MD004, ul-style)
Unordered list style
145-145: Expected: asterisk; Actual: dash (MD004, ul-style)
Unordered list style
146-146: Expected: asterisk; Actual: dash (MD004, ul-style)
Unordered list style
147-147: Expected: asterisk; Actual: dash (MD004, ul-style)
Unordered list style
148-148: Expected: asterisk; Actual: dash (MD004, ul-style)
Unordered list style
149-149: Expected: asterisk; Actual: dash (MD004, ul-style)
Unordered list style
150-150: Expected: asterisk; Actual: dash (MD004, ul-style)
Unordered list style
151-151: Expected: asterisk; Actual: dash (MD004, ul-style)
Unordered list style
152-152: Expected: asterisk; Actual: dash (MD004, ul-style)
Unordered list style
153-153: Expected: asterisk; Actual: dash (MD004, ul-style)
Unordered list style
223-223: Expected: asterisk; Actual: dash (MD004, ul-style)
Unordered list style
224-224: Expected: asterisk; Actual: dash (MD004, ul-style)
Unordered list style
225-225: Expected: asterisk; Actual: dash (MD004, ul-style)
Unordered list style
226-226: Expected: asterisk; Actual: dash (MD004, ul-style)
Unordered list style
227-227: Expected: asterisk; Actual: dash (MD004, ul-style)
Unordered list style
228-228: Expected: asterisk; Actual: dash (MD004, ul-style)
Unordered list style
229-229: Expected: asterisk; Actual: dash (MD004, ul-style)
Unordered list style
230-230: Expected: asterisk; Actual: dash (MD004, ul-style)
Unordered list style
231-231: Expected: asterisk; Actual: dash (MD004, ul-style)
Unordered list style
232-232: Expected: asterisk; Actual: dash (MD004, ul-style)
Unordered list style
233-233: Expected: asterisk; Actual: dash (MD004, ul-style)
Unordered list style
234-234: Expected: asterisk; Actual: dash (MD004, ul-style)
Unordered list style
235-235: Expected: asterisk; Actual: dash (MD004, ul-style)
Unordered list style
236-236: Expected: asterisk; Actual: dash (MD004, ul-style)
Unordered list style
237-237: Expected: asterisk; Actual: dash (MD004, ul-style)
Unordered list style
238-238: Expected: asterisk; Actual: dash (MD004, ul-style)
Unordered list style
239-239: Expected: asterisk; Actual: dash (MD004, ul-style)
Unordered list style
240-240: Expected: asterisk; Actual: dash (MD004, ul-style)
Unordered list style
241-241: Expected: asterisk; Actual: dash (MD004, ul-style)
Unordered list style
242-242: Expected: asterisk; Actual: dash (MD004, ul-style)
Unordered list style
243-243: Expected: asterisk; Actual: dash (MD004, ul-style)
Unordered list style
244-244: Expected: asterisk; Actual: dash (MD004, ul-style)
Unordered list style
245-245: Expected: asterisk; Actual: dash (MD004, ul-style)
Unordered list style
246-246: Expected: asterisk; Actual: dash (MD004, ul-style)
Unordered list style
247-247: Expected: asterisk; Actual: dash (MD004, ul-style)
Unordered list style
248-248: Expected: asterisk; Actual: dash (MD004, ul-style)
Unordered list style
249-249: Expected: asterisk; Actual: dash (MD004, ul-style)
Unordered list style
250-250: Expected: asterisk; Actual: dash (MD004, ul-style)
Unordered list style
251-251: Expected: asterisk; Actual: dash (MD004, ul-style)
Unordered list style
252-252: Expected: asterisk; Actual: dash (MD004, ul-style)
Unordered list style
253-253: Expected: asterisk; Actual: dash (MD004, ul-style)
Unordered list style
254-254: Expected: asterisk; Actual: dash (MD004, ul-style)
Unordered list style
283-283: Expected: asterisk; Actual: dash (MD004, ul-style)
Unordered list style
284-284: Expected: asterisk; Actual: dash (MD004, ul-style)
Unordered list style
285-285: Expected: asterisk; Actual: dash (MD004, ul-style)
Unordered list style
362-362: Expected: asterisk; Actual: dash (MD004, ul-style)
Unordered list style
363-363: Expected: asterisk; Actual: dash (MD004, ul-style)
Unordered list style
364-364: Expected: asterisk; Actual: dash (MD004, ul-style)
Unordered list style
365-365: Expected: asterisk; Actual: dash (MD004, ul-style)
Unordered list style
366-366: Expected: asterisk; Actual: dash (MD004, ul-style)
Unordered list style
367-367: Expected: asterisk; Actual: dash (MD004, ul-style)
Unordered list style
368-368: Expected: asterisk; Actual: dash (MD004, ul-style)
Unordered list style
369-369: Expected: asterisk; Actual: dash (MD004, ul-style)
Unordered list style
370-370: Expected: asterisk; Actual: dash (MD004, ul-style)
Unordered list style
371-371: Expected: asterisk; Actual: dash (MD004, ul-style)
Unordered list style
372-372: Expected: asterisk; Actual: dash (MD004, ul-style)
Unordered list style
373-373: Expected: asterisk; Actual: dash (MD004, ul-style)
Unordered list style
374-374: Expected: asterisk; Actual: dash (MD004, ul-style)
Unordered list style
375-375: Expected: asterisk; Actual: dash (MD004, ul-style)
Unordered list style
376-376: Expected: asterisk; Actual: dash (MD004, ul-style)
Unordered list style
377-377: Expected: asterisk; Actual: dash (MD004, ul-style)
Unordered list style
378-378: Expected: asterisk; Actual: dash (MD004, ul-style)
Unordered list style
379-379: Expected: asterisk; Actual: dash (MD004, ul-style)
Unordered list style
380-380: Expected: asterisk; Actual: dash (MD004, ul-style)
Unordered list style
381-381: Expected: asterisk; Actual: dash (MD004, ul-style)
Unordered list style
382-382: Expected: asterisk; Actual: dash (MD004, ul-style)
Unordered list style
383-383: Expected: asterisk; Actual: dash (MD004, ul-style)
Unordered list style
384-384: Expected: asterisk; Actual: dash (MD004, ul-style)
Unordered list style
740-740: Expected: asterisk; Actual: dash (MD004, ul-style)
Unordered list style
748-748: Expected: asterisk; Actual: dash (MD004, ul-style)
Unordered list style
793-793: Expected: asterisk; Actual: dash (MD004, ul-style)
Unordered list style
801-801: Expected: asterisk; Actual: dash (MD004, ul-style)
Unordered list style
847-847: Expected: 1; Actual: 2 (MD012, no-multiple-blanks)
Multiple consecutive blank lines
1248-1248: Expected: 1; Actual: 2 (MD012, no-multiple-blanks)
Multiple consecutive blank lines
1265-1265: Expected: 1; Actual: 2 (MD012, no-multiple-blanks)
Multiple consecutive blank lines
1317-1317: Expected: 1; Actual: 2 (MD012, no-multiple-blanks)
Multiple consecutive blank lines
1421-1421: Expected: 1; Actual: 2 (MD012, no-multiple-blanks)
Multiple consecutive blank lines
1243-1243: null (MD019, no-multiple-space-atx)
Multiple spaces after hash on atx style heading
783-783: null (MD024, no-duplicate-heading)
Multiple headings with the same content
824-824: null (MD024, no-duplicate-heading)
Multiple headings with the same content
1118-1118: null (MD024, no-duplicate-heading)
Multiple headings with the same content
258-258: null (MD034, no-bare-urls)
Bare URL used
289-289: null (MD034, no-bare-urls)
Bare URL used
388-388: null (MD034, no-bare-urls)
Bare URL used
438-438: null (MD034, no-bare-urls)
Bare URL used
450-450: null (MD034, no-bare-urls)
Bare URL used
466-466: null (MD034, no-bare-urls)
Bare URL used
537-537: null (MD034, no-bare-urls)
Bare URL used
548-548: null (MD034, no-bare-urls)
Bare URL used
556-556: null (MD034, no-bare-urls)
Bare URL used
592-592: null (MD034, no-bare-urls)
Bare URL used
617-617: null (MD034, no-bare-urls)
Bare URL used
629-629: null (MD034, no-bare-urls)
Bare URL used
675-675: null (MD034, no-bare-urls)
Bare URL used
676-676: null (MD034, no-bare-urls)
Bare URL used
677-677: null (MD034, no-bare-urls)
Bare URL used
678-678: null (MD034, no-bare-urls)
Bare URL used
679-679: null (MD034, no-bare-urls)
Bare URL used
726-726: null (MD034, no-bare-urls)
Bare URL used
767-767: null (MD034, no-bare-urls)
Bare URL used
820-820: null (MD034, no-bare-urls)
Bare URL used
933-933: null (MD034, no-bare-urls)
Bare URL used
975-975: null (MD037, no-space-in-emphasis)
Spaces inside emphasis markers
975-975: null (MD037, no-space-in-emphasis)
Spaces inside emphasis markers
371-371: null (MD038, no-space-in-code)
Spaces inside code span elements
Additional comments not posted (15)
app/prefix.go (1)
29-47: The changes to theSetConfigfunction align with the PR's objectives to correct network configurations based on the environment. Good use of conditional logic to handle both mainnet and testnet configurations.cmd/provenanced/config/interceptor.go (1)
59-59: The addition of handling theEnvTypeFlaginInterceptConfigsPreRunHandleris well-placed and aligns with the objective to differentiate environment-specific configurations.app/prefix_test/prefix_test.go (1)
33-157: The updatedTestSetConfigfunction thoroughly tests various scenarios including mainnet, testnet, and sealed configurations. Excellent coverage and use of assertions to ensure the configuration behaves as expected.cmd/provenanced/cmd/init.go (1)
81-81: The update to useprovconfig.EnvTypeFlagfor checking the testnet status in theInitfunction aligns with the PR's objectives to handle testnet configurations correctly.cmd/provenanced/cmd/root.go (4)
107-107: Ensure the proper handling of thetestnetflag for setting configurations.This change correctly fetches the
testnetstatus using the new flag configuration and sets the app configuration accordingly.
154-154: Ensure the new flagEnvTypeFlagfor configuring environment type is correctly added and defaults tomainnet.The addition of this flag improves clarity and configurability for setting the environment type.
124-124: Verify thatCoinTypeMainNetis correctly set as the default coin type.Verification successful
The
CoinTypeMainNetis defined and used in the following locations:
cmd/provenanced/cmd/root.go: Usage in a configuration setting.app/prefix.go: Defined and set as the default coin type (CoinTypeMainNet = 505andCoinType = CoinTypeMainNet).app/prefix_test/prefix_test.go: Used in test cases.This confirms that
CoinTypeMainNetis correctly set as the default coin type.
app/prefix.go
CoinTypeMainNet = 505CoinType = CoinTypeMainNetScripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Ensure that `CoinTypeMainNet` is set as the default coin type unless overridden. # Test: Search for the usage of `CoinTypeMainNet` and ensure it's used as the default. rg --type python $'CoinTypeMainNet'Length of output: 65
Script:
#!/bin/bash # Description: Ensure that `CoinTypeMainNet` is set as the default coin type unless overridden. # Test: Search for the definition and usage of `CoinTypeMainNet` within Go files. rg 'CoinTypeMainNet' --type goLength of output: 402
112-112: Verify the dynamic setting ofCoinTypebased on the network type.app/upgrades.go (2)
55-55: Ensure the handling of added modulescrisistypes.ModuleNameandconsensusparamtypes.ModuleNameduring upgrades.These additions are crucial for the upgrade process to handle specific module parameters and states correctly.
Also applies to: 94-94
7-7: Verify the correct usage and necessity of new importssdkmathandconsensusparamtypes.Also applies to: 15-15
Verification successful
The imports
sdkmathandconsensusparamtypesare indeed used in the fileapp/upgrades.go. Thesdkmathimport is used to declare a variablemaxSupply, and theconsensusparamtypesimport is used within an array of added module names.
sdkmathis used on line:var maxSupply sdkmath.Intconsensusparamtypesis used in:Added: []string{crisistypes.ModuleName, consensusparamtypes.ModuleName}Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify that the new imports are used appropriately throughout the file. # Test: Search for the usage of `sdkmath` and `consensusparamtypes` to ensure they are necessary and correctly used. rg --type go $'sdkmath|consensusparamtypes' ./app/upgrades.goLength of output: 343
app/app.go (5)
93-93: The update togovtypesv1andgovtypesv1beta1reflects a version change or restructuring in the governance module types. Confirm that all governance-related functionality is tested to ensure compatibility with these new type versions.Verification successful
The search results indicate that both
govtypesv1andgovtypesv1beta1are extensively used across various test files. This suggests that there is a significant amount of test coverage for the updated governance types.Given the comprehensive presence of these types in the test files, it is reasonable to conclude that the governance-related functionality is well-tested and should be compatible with the new type versions.
Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify that the updated governance types are correctly used. # Test: Search for usage of the updated governance types. Expect: Correct usage and compatibility. rg --type go $'govtypesv1|govtypesv1beta1'Length of output: 5169
Script:
#!/bin/bash # Searching for test functions that involve govtypesv1 and govtypesv1beta1 to ensure comprehensive test coverage. # Find all test files test_files=$(fd --type f --extension go --glob '*_test.go') # Search for test functions involving govtypesv1 and govtypesv1beta1 in test files for file in $test_files; do echo "File: $file" grep -E 'govtypesv1|govtypesv1beta1' $file doneLength of output: 63567
13-13: The import ofgithub.com/CosmWasm/wasmd/x/wasm/migrations/v2is new. Ensure that all dependencies and references to this module are correctly updated elsewhere in the codebase as needed.Verification successful
The import
github.com/CosmWasm/wasmd/x/wasm/migrations/v2is correctly used inapp/app.goto set up a key table for the wasm module parameters.
app/app.go
paramsKeeper.Subspace(wasmtypes.ModuleName).WithKeyTable(wasmv2.ParamKeyTable())Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify that the new wasm migration module is correctly used. # Test: Search for references to the new wasm migration module. Expect: Correct usage in relevant parts of the code. rg --type go $'github.com/CosmWasm/wasmd/x/wasm/migrations/v2'Length of output: 129
Script:
#!/bin/bash # Description: Extract the usage of the imported `wasmv2` within `app/app.go`. # Test: Search for references to `wasmv2` within `app/app.go`. Expect: Correct usage in relevant parts of the file. rg --type go 'wasmv2' app/app.goLength of output: 192
339-340: TheAddressCodecandValidatorAddressCodechave been modified to use the newBech32Codecstructure. This change will affect how addresses are handled throughout the application. Ensure thorough testing, especially in modules that perform address validation or manipulation.Verification successful
The refined search confirms that the
Bech32Codecis used inapp/app.goforAddressCodecandValidatorAddressCodec. Additionally, the presence ofBech32Codecinx/quarantine/client/testutil/common_test.gosuggests that there are tests covering its usage.
app/app.go, lines 339-340: Usage ofaddress.Bech32CodecforAddressCodecandValidatorAddressCodec.x/quarantine/client/testutil/common_test.go: Test file containingBech32Codec.The changes to
Bech32Codecappear to be correctly integrated and tested.Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify that the new address codec is correctly used. # Test: Search for usage of the new address codec. Expect: Correct usage and compatibility. rg --type go $'Bech32Codec'Length of output: 822
Script:
#!/bin/bash # Description: Verify the functions and methods using Bech32Codec and check for corresponding test cases. # List functions and methods using Bech32Codec echo "Functions and methods using Bech32Codec:" ast-grep --lang go --pattern $'func $_($_) $_ { $$$ address.Bech32Codec{$_} $$$ }' # List test files to ensure coverage echo "Test files containing Bech32Codec:" rg --type go --glob '*_test.go' 'Bech32Codec'Length of output: 429
Script:
#!/bin/bash # Description: Refine the search for functions and methods using Bech32Codec and ensure comprehensive test coverage. # Refine the search pattern to capture more varied usages of Bech32Codec echo "Refined search for functions and methods using Bech32Codec:" ast-grep --lang go --pattern $'$_ := address.Bech32Codec{$_}' ast-grep --lang go --pattern $'$_ = address.Bech32Codec{$_}' ast-grep --lang go --pattern $'address.Bech32Codec{$_}' # List test files to ensure comprehensive coverage echo "Test files containing Bech32Codec (refined search):" rg --type go --glob '*_test.go' 'Bech32Codec'Length of output: 774
437-437: TheAccountKeeperinitialization now includes new parameters for address codec and prefix handling. Ensure that these changes are reflected whereverAccountKeeperis utilized, and verify that the new parameters are correctly integrated and functioning as expected.
1336-1354: The registration of parameter subspaces has been updated to include new modules and key tables. This is a critical area as it involves configuration settings that affect the entire application. Verify that these settings are correct and that the subspaces are appropriately configured without any missing parameters.Verification successful
The output lists multiple instances of
WithKeyTableconfigurations inapp/app.goandapp/upgrades.go. To ensure correctness, we need to verify that each module's subspace is registered with the appropriate key table and that no required modules are missing.Modules and Key Tables to Verify:
authtypes.ModuleNamewithauthtypes.ParamKeyTable()stakingtypes.ModuleNamewithstakingtypes.ParamKeyTable()minttypes.ModuleNamewithminttypes.ParamKeyTable()distrtypes.ModuleNamewithdistrtypes.ParamKeyTable()slashingtypes.ModuleNamewithslashingtypes.ParamKeyTable()govtypes.ModuleNamewithgovtypesv1.ParamKeyTable()crisistypes.ModuleNamewithcrisistypes.ParamKeyTable()wasmtypes.ModuleNamewithwasmv2.ParamKeyTable()ibcexported.ModuleNamewithkeyTable(custom configuration)ibctransfertypes.ModuleNamewithibctransfertypes.ParamKeyTable()icahosttypes.SubModuleNamewithicahosttypes.ParamKeyTable()icqtypes.ModuleNamewithicqtypes.ParamKeyTable()- Additional configurations in
app/upgrades.go:
baseapp.Paramspacewithparamstypes.ConsensusParamsKeyTable()attributetypes.ModuleNamewithattributetypes.ParamKeyTable()markertypes.ModuleNamewithmarkertypes.ParamKeyTable()nametypes.ModuleNamewithnametypes.ParamKeyTable()msgfeestypes.ModuleNamewithmsgfeestypes.ParamKeyTable()ibchookstypes.ModuleNamewithibchookstypes.ParamKeyTable()Next, we need to ensure that each of these key tables is correctly defined and used. Let's verify the presence and correctness of these key tables in the codebase.
The gathered information confirms the presence and correctness of
ParamKeyTablefunctions for the relevant modules. Each module'sParamKeyTableis correctly defined and used in the codebase, and the customkeyTableforibcexported.ModuleNameis also verified.Verified Modules and Key Tables:
authtypes.ModuleNamewithauthtypes.ParamKeyTable()stakingtypes.ModuleNamewithstakingtypes.ParamKeyTable()minttypes.ModuleNamewithminttypes.ParamKeyTable()distrtypes.ModuleNamewithdistrtypes.ParamKeyTable()slashingtypes.ModuleNamewithslashingtypes.ParamKeyTable()govtypes.ModuleNamewithgovtypesv1.ParamKeyTable()crisistypes.ModuleNamewithcrisistypes.ParamKeyTable()wasmtypes.ModuleNamewithwasmv2.ParamKeyTable()ibcexported.ModuleNamewithkeyTable(custom configuration)ibctransfertypes.ModuleNamewithibctransfertypes.ParamKeyTable()icahosttypes.SubModuleNamewithicahosttypes.ParamKeyTable()icqtypes.ModuleNamewithicqtypes.ParamKeyTable()- Additional configurations in
app/upgrades.go:
baseapp.Paramspacewithparamstypes.ConsensusParamsKeyTable()attributetypes.ModuleNamewithattributetypes.ParamKeyTable()markertypes.ModuleNamewithmarkertypes.ParamKeyTable()nametypes.ModuleNamewithnametypes.ParamKeyTable()msgfeestypes.ModuleNamewithmsgfeestypes.ParamKeyTable()ibchookstypes.ModuleNamewithibchookstypes.ParamKeyTable()All required modules have their subspaces registered with the correct key tables, and no configurations are missing.
Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify the correct configuration of parameter subspaces. # Test: Search for parameter subspace configurations. Expect: Correct configuration and no missing parameters. rg --type go $'WithKeyTable'Length of output: 2174
Script:
#!/bin/bash # Verify the definitions of ParamKeyTable functions for each module # Search for the definition of ParamKeyTable in the relevant module files rg --type go 'func ParamKeyTable' --files-with-matches | xargs -I {} sh -c 'echo "File: {}"; rg "func ParamKeyTable" {}' # Special case: Verify the custom keyTable for ibcexported.ModuleName rg --type go 'keyTable := ibcclienttypes.ParamKeyTable()' --files-with-matches | xargs -I {} sh -c 'echo "File: {}"; rg "keyTable := ibcclienttypes.ParamKeyTable()" {}'Length of output: 808
# Conflicts: # app/upgrades.go
…s added in a different PR).
…onna have to just live with our forks for the async-icq and ibc-go libraries for now.
There was a problem hiding this comment.
Actionable comments posted: 0
Outside diff range and nitpick comments (6)
CHANGELOG.md (6)
Line range hint
130-152: Consider using asterisks (*) instead of dashes (-) for list items to maintain consistency in markdown list styles.- - 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 `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))Also applies to: 224-256, 284-286, 363-384, 741-741, 749-749, 794-794, 802-802, 848-848, 1249-1249, 1266-1266, 1318-1318, 1422-1422
Line range hint
1244-1244: There is a markdown lint issue with multiple spaces after the hash in an ATX style heading. Consider removing extra spaces.- ## Features + ## Features
Line range hint
784-784: Multiple headings with the same content can confuse readers and should be avoided unless necessary for clarity or emphasis.Consider revising the headings to be more specific or combining sections under a single heading if the content is related.
Also applies to: 825-825, 1119-1119
Line range hint
259-259: Bare URLs were found in the document. It's a good practice to hyperlink text instead of using bare URLs for better readability and aesthetics.Consider using markdown links, for example:
[Provenance](https://www.provenance.io)Also applies to: 290-290, 389-389, 439-439, 451-451, 467-467, 538-538, 549-549, 557-557, 593-593, 618-618, 630-630, 676-676, 677-677, 678-678, 679-679, 680-680, 727-727, 768-768, 821-821, 934-934
Line range hint
976-976: Spaces inside emphasis markers can lead to rendering issues. Please remove any leading or trailing spaces within emphasis markers.- ** Provenance ** + **Provenance**
Line range hint
372-372: Spaces inside code span elements can cause formatting issues. Please remove any leading or trailing spaces within code span elements.- ` Code ` + `Code`
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (4)
- CHANGELOG.md (1 hunks)
- app/app.go (10 hunks)
- app/upgrades.go (8 hunks)
- go.mod (1 hunks)
Files skipped from review due to trivial changes (1)
- go.mod
Files skipped from review as they are similar to previous changes (1)
- app/app.go
Additional context used
Learnings (1)
app/upgrades.go (1)
User: SpicyLemon PR: provenance-io/provenance#2033 File: app/upgrades.go:303-310 Timestamp: 2024-06-13T21:10:37.185Z Learning: There are no rollback mechanisms used in the migration functions of the `app/upgrades.go` file, and error handling is managed by logging and returning errors as per the project's standards.
Markdownlint
CHANGELOG.md
130-130: Expected: asterisk; Actual: dash (MD004, ul-style)
Unordered list style
131-131: Expected: asterisk; Actual: dash (MD004, ul-style)
Unordered list style
132-132: Expected: asterisk; Actual: dash (MD004, ul-style)
Unordered list style
133-133: Expected: asterisk; Actual: dash (MD004, ul-style)
Unordered list style
134-134: Expected: asterisk; Actual: dash (MD004, ul-style)
Unordered list style
135-135: Expected: asterisk; Actual: dash (MD004, ul-style)
Unordered list style
136-136: Expected: asterisk; Actual: dash (MD004, ul-style)
Unordered list style
137-137: Expected: asterisk; Actual: dash (MD004, ul-style)
Unordered list style
138-138: Expected: asterisk; Actual: dash (MD004, ul-style)
Unordered list style
139-139: Expected: asterisk; Actual: dash (MD004, ul-style)
Unordered list style
140-140: Expected: asterisk; Actual: dash (MD004, ul-style)
Unordered list style
141-141: Expected: asterisk; Actual: dash (MD004, ul-style)
Unordered list style
142-142: Expected: asterisk; Actual: dash (MD004, ul-style)
Unordered list style
143-143: Expected: asterisk; Actual: dash (MD004, ul-style)
Unordered list style
144-144: Expected: asterisk; Actual: dash (MD004, ul-style)
Unordered list style
145-145: Expected: asterisk; Actual: dash (MD004, ul-style)
Unordered list style
146-146: Expected: asterisk; Actual: dash (MD004, ul-style)
Unordered list style
147-147: Expected: asterisk; Actual: dash (MD004, ul-style)
Unordered list style
148-148: Expected: asterisk; Actual: dash (MD004, ul-style)
Unordered list style
149-149: Expected: asterisk; Actual: dash (MD004, ul-style)
Unordered list style
150-150: Expected: asterisk; Actual: dash (MD004, ul-style)
Unordered list style
151-151: Expected: asterisk; Actual: dash (MD004, ul-style)
Unordered list style
152-152: Expected: asterisk; Actual: dash (MD004, ul-style)
Unordered list style
153-153: Expected: asterisk; Actual: dash (MD004, ul-style)
Unordered list style
154-154: Expected: asterisk; Actual: dash (MD004, ul-style)
Unordered list style
224-224: Expected: asterisk; Actual: dash (MD004, ul-style)
Unordered list style
225-225: Expected: asterisk; Actual: dash (MD004, ul-style)
Unordered list style
226-226: Expected: asterisk; Actual: dash (MD004, ul-style)
Unordered list style
227-227: Expected: asterisk; Actual: dash (MD004, ul-style)
Unordered list style
228-228: Expected: asterisk; Actual: dash (MD004, ul-style)
Unordered list style
229-229: Expected: asterisk; Actual: dash (MD004, ul-style)
Unordered list style
230-230: Expected: asterisk; Actual: dash (MD004, ul-style)
Unordered list style
231-231: Expected: asterisk; Actual: dash (MD004, ul-style)
Unordered list style
232-232: Expected: asterisk; Actual: dash (MD004, ul-style)
Unordered list style
233-233: Expected: asterisk; Actual: dash (MD004, ul-style)
Unordered list style
234-234: Expected: asterisk; Actual: dash (MD004, ul-style)
Unordered list style
235-235: Expected: asterisk; Actual: dash (MD004, ul-style)
Unordered list style
236-236: Expected: asterisk; Actual: dash (MD004, ul-style)
Unordered list style
237-237: Expected: asterisk; Actual: dash (MD004, ul-style)
Unordered list style
238-238: Expected: asterisk; Actual: dash (MD004, ul-style)
Unordered list style
239-239: Expected: asterisk; Actual: dash (MD004, ul-style)
Unordered list style
240-240: Expected: asterisk; Actual: dash (MD004, ul-style)
Unordered list style
241-241: Expected: asterisk; Actual: dash (MD004, ul-style)
Unordered list style
242-242: Expected: asterisk; Actual: dash (MD004, ul-style)
Unordered list style
243-243: Expected: asterisk; Actual: dash (MD004, ul-style)
Unordered list style
244-244: Expected: asterisk; Actual: dash (MD004, ul-style)
Unordered list style
245-245: Expected: asterisk; Actual: dash (MD004, ul-style)
Unordered list style
246-246: Expected: asterisk; Actual: dash (MD004, ul-style)
Unordered list style
247-247: Expected: asterisk; Actual: dash (MD004, ul-style)
Unordered list style
248-248: Expected: asterisk; Actual: dash (MD004, ul-style)
Unordered list style
249-249: Expected: asterisk; Actual: dash (MD004, ul-style)
Unordered list style
250-250: Expected: asterisk; Actual: dash (MD004, ul-style)
Unordered list style
251-251: Expected: asterisk; Actual: dash (MD004, ul-style)
Unordered list style
252-252: Expected: asterisk; Actual: dash (MD004, ul-style)
Unordered list style
253-253: Expected: asterisk; Actual: dash (MD004, ul-style)
Unordered list style
254-254: Expected: asterisk; Actual: dash (MD004, ul-style)
Unordered list style
255-255: Expected: asterisk; Actual: dash (MD004, ul-style)
Unordered list style
284-284: Expected: asterisk; Actual: dash (MD004, ul-style)
Unordered list style
285-285: Expected: asterisk; Actual: dash (MD004, ul-style)
Unordered list style
286-286: Expected: asterisk; Actual: dash (MD004, ul-style)
Unordered list style
363-363: Expected: asterisk; Actual: dash (MD004, ul-style)
Unordered list style
364-364: Expected: asterisk; Actual: dash (MD004, ul-style)
Unordered list style
365-365: Expected: asterisk; Actual: dash (MD004, ul-style)
Unordered list style
366-366: Expected: asterisk; Actual: dash (MD004, ul-style)
Unordered list style
367-367: Expected: asterisk; Actual: dash (MD004, ul-style)
Unordered list style
368-368: Expected: asterisk; Actual: dash (MD004, ul-style)
Unordered list style
369-369: Expected: asterisk; Actual: dash (MD004, ul-style)
Unordered list style
370-370: Expected: asterisk; Actual: dash (MD004, ul-style)
Unordered list style
371-371: Expected: asterisk; Actual: dash (MD004, ul-style)
Unordered list style
372-372: Expected: asterisk; Actual: dash (MD004, ul-style)
Unordered list style
373-373: Expected: asterisk; Actual: dash (MD004, ul-style)
Unordered list style
374-374: Expected: asterisk; Actual: dash (MD004, ul-style)
Unordered list style
375-375: Expected: asterisk; Actual: dash (MD004, ul-style)
Unordered list style
376-376: Expected: asterisk; Actual: dash (MD004, ul-style)
Unordered list style
377-377: Expected: asterisk; Actual: dash (MD004, ul-style)
Unordered list style
378-378: Expected: asterisk; Actual: dash (MD004, ul-style)
Unordered list style
379-379: Expected: asterisk; Actual: dash (MD004, ul-style)
Unordered list style
380-380: Expected: asterisk; Actual: dash (MD004, ul-style)
Unordered list style
381-381: Expected: asterisk; Actual: dash (MD004, ul-style)
Unordered list style
382-382: Expected: asterisk; Actual: dash (MD004, ul-style)
Unordered list style
383-383: Expected: asterisk; Actual: dash (MD004, ul-style)
Unordered list style
384-384: Expected: asterisk; Actual: dash (MD004, ul-style)
Unordered list style
385-385: Expected: asterisk; Actual: dash (MD004, ul-style)
Unordered list style
741-741: Expected: asterisk; Actual: dash (MD004, ul-style)
Unordered list style
749-749: Expected: asterisk; Actual: dash (MD004, ul-style)
Unordered list style
794-794: Expected: asterisk; Actual: dash (MD004, ul-style)
Unordered list style
802-802: Expected: asterisk; Actual: dash (MD004, ul-style)
Unordered list style
848-848: Expected: 1; Actual: 2 (MD012, no-multiple-blanks)
Multiple consecutive blank lines
1249-1249: Expected: 1; Actual: 2 (MD012, no-multiple-blanks)
Multiple consecutive blank lines
1266-1266: Expected: 1; Actual: 2 (MD012, no-multiple-blanks)
Multiple consecutive blank lines
1318-1318: Expected: 1; Actual: 2 (MD012, no-multiple-blanks)
Multiple consecutive blank lines
1422-1422: Expected: 1; Actual: 2 (MD012, no-multiple-blanks)
Multiple consecutive blank lines
1244-1244: null (MD019, no-multiple-space-atx)
Multiple spaces after hash on atx style heading
784-784: null (MD024, no-duplicate-heading)
Multiple headings with the same content
825-825: null (MD024, no-duplicate-heading)
Multiple headings with the same content
1119-1119: null (MD024, no-duplicate-heading)
Multiple headings with the same content
259-259: null (MD034, no-bare-urls)
Bare URL used
290-290: null (MD034, no-bare-urls)
Bare URL used
389-389: null (MD034, no-bare-urls)
Bare URL used
439-439: null (MD034, no-bare-urls)
Bare URL used
451-451: null (MD034, no-bare-urls)
Bare URL used
467-467: null (MD034, no-bare-urls)
Bare URL used
538-538: null (MD034, no-bare-urls)
Bare URL used
549-549: null (MD034, no-bare-urls)
Bare URL used
557-557: null (MD034, no-bare-urls)
Bare URL used
593-593: null (MD034, no-bare-urls)
Bare URL used
618-618: null (MD034, no-bare-urls)
Bare URL used
630-630: null (MD034, no-bare-urls)
Bare URL used
676-676: null (MD034, no-bare-urls)
Bare URL used
677-677: null (MD034, no-bare-urls)
Bare URL used
678-678: null (MD034, no-bare-urls)
Bare URL used
679-679: null (MD034, no-bare-urls)
Bare URL used
680-680: null (MD034, no-bare-urls)
Bare URL used
727-727: null (MD034, no-bare-urls)
Bare URL used
768-768: null (MD034, no-bare-urls)
Bare URL used
821-821: null (MD034, no-bare-urls)
Bare URL used
934-934: null (MD034, no-bare-urls)
Bare URL used
976-976: null (MD037, no-space-in-emphasis)
Spaces inside emphasis markers
976-976: null (MD037, no-space-in-emphasis)
Spaces inside emphasis markers
372-372: null (MD038, no-space-in-code)
Spaces inside code span elements
Additional comments not posted (5)
app/upgrades.go (5)
304-311: The migration of consensus parameters is logged extensively, which is good for tracking. However, ensure that the migration process is tested in environments that closely mimic production to prevent any unforeseen issues during the actual upgrade.
393-394: The update to useDefaultOSLocatorParamsinmigrateMetadataOSLocatorParamssimplifies the configuration process by using default values. This change should make the configuration more robust and less prone to errors. However, verify that the default values meet all operational requirements.
463-463: The migration of IBC hooks parameters now includes an allowlist, which is a critical security feature. It's important to ensure that this allowlist is maintained rigorously and updated according to security policies and operational needs.
Line range hint
56-95: The update to theupgradesmap for bothumber-rc1andumberincludes the addition of new modules (crisistypes,circuittypes,consensusparamtypes) and the removal of therewardmodule. Ensure that the dependencies and interactions of the new modules are thoroughly tested, especially since the removal of a module can have significant ripple effects.Consider documenting the reasons for these specific additions and deletions within the upgrade map, especially if they relate to the issues addressed in this PR.
Verification successful
The
rgcommand output indicates several occurrences of the term "reward" across different files. These references are found inapp/upgrades.go,app/export.go, andapp/app_test.go. To determine if these references pertain to the removedrewardmodule or if they are part of other contexts (like comments or test cases), we need to delve deeper into the specific lines and contexts where "reward" is mentioned.To proceed, we should examine the contexts of these references in the
app/export.goandapp/app_test.gofiles to confirm whether they are related to the removedrewardmodule or not.
The references to "reward" in
app/export.goandapp/app_test.goare related to the distribution logic and test cases, not the removedrewardmodule. Therefore, the removal of therewardmodule does not impact these references.
app/export.go: The references are part of the logic to withdraw delegator rewards, clear validator historical rewards, and handle unwithdrawn outstanding reward fractions.app/app_test.go: The references are part of test cases for various reward-related events.Thus, the removal of the
rewardmodule appears to be safe and does not leave any residual dependencies.Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify that the new modules are properly integrated and that there are no residual dependencies on the `reward` module. # Test: Search for references to the `reward` module. Expect: No occurrences. rg --type go $'reward'Length of output: 930
Script:
#!/bin/bash # Description: Examine the lines in app/export.go and app/app_test.go where "reward" is mentioned to determine the context. # Display lines with "reward" in app/export.go rg --type go --context 5 'reward' app/export.go # Display lines with "reward" in app/app_test.go rg --type go --context 5 'reward' app/app_test.goLength of output: 2358
379-381: The migration of theMaxSupplyparameter inmigrateMarkerParamsfunction has been updated to usesdkmath.Int. This is a good change for type safety and precision. However, ensure that all consuming functions and modules are compatible with this new type.Verification successful
The migration of the
MaxSupplyparameter to usesdkmath.Intappears to be well-integrated across various parts of the codebase, including tests, parameters, protobuf messages, simulations, events, and keeper methods. The search results confirm thatsdkmath.Intis utilized correctly in these contexts.
- Tests:
sdkmath.Intis used in multiple test files.- Params:
sdkmath.Intis correctly used in parameter definitions.- Protobuf Messages:
sdkmath.Intis handled in protobuf message definitions.- Simulations:
sdkmath.Intis used in simulation files.- Events:
sdkmath.Intis used in event definitions.- Keeper Methods:
sdkmath.Intis used in keeper method definitions.Given these findings, it appears that the migration to
sdkmath.Inthas been successfully applied throughout the codebase.Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify that all modules consuming `MaxSupply` handle the new `sdkmath.Int` type correctly. # Test: Search for usage of `MaxSupply`. Expect: Proper handling of `sdkmath.Int` type. rg --type go $'MaxSupply'Length of output: 5601
Script:
#!/bin/bash # Verify handling of `sdkmath.Int` in tests rg --type go 'MaxSupply' | grep '_test.go' | xargs -I {} sh -c 'echo "File: {}\n"; rg --type go "sdkmath.Int" {}' # Verify handling of `sdkmath.Int` in params rg --type go 'MaxSupply' | grep 'params.go' | xargs -I {} sh -c 'echo "File: {}\n"; rg --type go "sdkmath.Int" {}' # Verify handling of `sdkmath.Int` in protobuf messages rg --type go 'MaxSupply' | grep 'marker.pb.go' | xargs -I {} sh -c 'echo "File: {}\n"; rg --type go "sdkmath.Int" {}' # Verify handling of `sdkmath.Int` in simulations rg --type go 'MaxSupply' | grep 'simulation' | xargs -I {} sh -c 'echo "File: {}\n"; rg --type go "sdkmath.Int" {}' # Verify handling of `sdkmath.Int` in events rg --type go 'MaxSupply' | grep 'events.go' | xargs -I {} sh -c 'echo "File: {}\n"; rg --type go "sdkmath.Int" {}' # Verify handling of `sdkmath.Int` in keeper methods rg --type go 'MaxSupply' | grep 'keeper' | xargs -I {} sh -c 'echo "File: {}\n"; rg --type go "sdkmath.Int" {}'Length of output: 31729
Script:
#!/bin/bash # Verify handling of `sdkmath.Int` in tests rg --type go 'MaxSupply' | grep '_test.go' | cut -d: -f1 | sort -u | xargs -I {} sh -c 'echo "File: {}\n"; rg "sdkmath.Int" {}' # Verify handling of `sdkmath.Int` in params rg --type go 'MaxSupply' | grep 'params.go' | cut -d: -f1 | sort -u | xargs -I {} sh -c 'echo "File: {}\n"; rg "sdkmath.Int" {}' # Verify handling of `sdkmath.Int` in protobuf messages rg --type go 'MaxSupply' | grep 'marker.pb.go' | cut -d: -f1 | sort -u | xargs -I {} sh -c 'echo "File: {}\n"; rg "sdkmath.Int" {}' # Verify handling of `sdkmath.Int` in simulations rg --type go 'MaxSupply' | grep 'simulation' | cut -d: -f1 | sort -u | xargs -I {} sh -c 'echo "File: {}\n"; rg "sdkmath.Int" {}' # Verify handling of `sdkmath.Int` in events rg --type go 'MaxSupply' | grep 'events.go' | cut -d: -f1 | sort -u | xargs -I {} sh -c 'echo "File: {}\n"; rg "sdkmath.Int" {}' # Verify handling of `sdkmath.Int` in keeper methods rg --type go 'MaxSupply' | grep 'keeper' | cut -d: -f1 | sort -u | xargs -I {} sh -c 'echo "File: {}\n"; rg "sdkmath.Int" {}'Length of output: 2163
There was a problem hiding this comment.
Actionable comments posted: 0
Outside diff range and nitpick comments (6)
CHANGELOG.md (6)
Line range hint
131-256: Please use asterisks (*) instead of dashes (-) for unordered lists to maintain consistency with Markdown standards.- - 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). ... - - Bump `cosmossdk.io/x/evidence` from 0.1.0 to 0.1.1 [#2026](https://github.com/provenance-io/provenance/pull/2026) + * Bump `cosmossdk.io/x/evidence` from 0.1.0 to 0.1.1 [#2026](https://github.com/provenance-io/provenance/pull/2026)
Line range hint
742-742: Use asterisks (*) instead of dashes (-) for unordered lists to maintain consistency with Markdown standards.- - 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 `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 `cosmossdk.io/api` from 0.7.4 to 0.7.5 ([#2025](https://github.com/provenance-io/provenance/pull/2025)) + * Bump `cosmossdk.io/api` from 0.7.4 to 0.7.5 ([#2025](https://github.com/provenance-io/provenance/pull/2025))Also applies to: 750-750, 795-795, 803-803
Line range hint
1245-1245: Please correct the multiple spaces after the hash in the ATX style heading to adhere to Markdown best practices.- ###### Features + ###### Features
Line range hint
785-785: Avoid using multiple headings with the same content to improve the structure and readability of the document.- ### Bug Fixes + ### Bug Fixes in Marker Module ... - ### Bug Fixes + ### Bug Fixes in Metadata Module ... - ### Bug Fixes + ### Bug Fixes in Attribute ModuleAlso applies to: 826-826, 1120-1120
Line range hint
260-260: Please enclose bare URLs in angle brackets to make them clickable and to comply with Markdown standards.- http://example.com + <http://example.com> ... - http://example.com + <http://example.com>Also applies to: 291-291, 390-390, 440-440, 452-452, 468-468, 539-539, 550-550, 558-558, 594-594, 619-619, 631-631, 677-677, 678-678, 679-679, 680-680, 681-681, 728-728, 769-769, 822-822, 935-935
Line range hint
977-977: Remove spaces inside emphasis markers to correct the Markdown formatting.- * this is not correct * + *this is correct*
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (3)
- CHANGELOG.md (1 hunks)
- app/upgrades.go (7 hunks)
- app/upgrades_test.go (2 hunks)
Files skipped from review as they are similar to previous changes (1)
- app/upgrades_test.go
Additional context used
Learnings (1)
app/upgrades.go (1)
User: SpicyLemon PR: provenance-io/provenance#2033 File: app/upgrades.go:303-310 Timestamp: 2024-06-13T21:10:37.185Z Learning: There are no rollback mechanisms used in the migration functions of the `app/upgrades.go` file, and error handling is managed by logging and returning errors as per the project's standards.
Markdownlint
CHANGELOG.md
131-131: Expected: asterisk; Actual: dash (MD004, ul-style)
Unordered list style
132-132: Expected: asterisk; Actual: dash (MD004, ul-style)
Unordered list style
133-133: Expected: asterisk; Actual: dash (MD004, ul-style)
Unordered list style
134-134: Expected: asterisk; Actual: dash (MD004, ul-style)
Unordered list style
135-135: Expected: asterisk; Actual: dash (MD004, ul-style)
Unordered list style
136-136: Expected: asterisk; Actual: dash (MD004, ul-style)
Unordered list style
137-137: Expected: asterisk; Actual: dash (MD004, ul-style)
Unordered list style
138-138: Expected: asterisk; Actual: dash (MD004, ul-style)
Unordered list style
139-139: Expected: asterisk; Actual: dash (MD004, ul-style)
Unordered list style
140-140: Expected: asterisk; Actual: dash (MD004, ul-style)
Unordered list style
141-141: Expected: asterisk; Actual: dash (MD004, ul-style)
Unordered list style
142-142: Expected: asterisk; Actual: dash (MD004, ul-style)
Unordered list style
143-143: Expected: asterisk; Actual: dash (MD004, ul-style)
Unordered list style
144-144: Expected: asterisk; Actual: dash (MD004, ul-style)
Unordered list style
145-145: Expected: asterisk; Actual: dash (MD004, ul-style)
Unordered list style
146-146: Expected: asterisk; Actual: dash (MD004, ul-style)
Unordered list style
147-147: Expected: asterisk; Actual: dash (MD004, ul-style)
Unordered list style
148-148: Expected: asterisk; Actual: dash (MD004, ul-style)
Unordered list style
149-149: Expected: asterisk; Actual: dash (MD004, ul-style)
Unordered list style
150-150: Expected: asterisk; Actual: dash (MD004, ul-style)
Unordered list style
151-151: Expected: asterisk; Actual: dash (MD004, ul-style)
Unordered list style
152-152: Expected: asterisk; Actual: dash (MD004, ul-style)
Unordered list style
153-153: Expected: asterisk; Actual: dash (MD004, ul-style)
Unordered list style
154-154: Expected: asterisk; Actual: dash (MD004, ul-style)
Unordered list style
155-155: Expected: asterisk; Actual: dash (MD004, ul-style)
Unordered list style
225-225: Expected: asterisk; Actual: dash (MD004, ul-style)
Unordered list style
226-226: Expected: asterisk; Actual: dash (MD004, ul-style)
Unordered list style
227-227: Expected: asterisk; Actual: dash (MD004, ul-style)
Unordered list style
228-228: Expected: asterisk; Actual: dash (MD004, ul-style)
Unordered list style
229-229: Expected: asterisk; Actual: dash (MD004, ul-style)
Unordered list style
230-230: Expected: asterisk; Actual: dash (MD004, ul-style)
Unordered list style
231-231: Expected: asterisk; Actual: dash (MD004, ul-style)
Unordered list style
232-232: Expected: asterisk; Actual: dash (MD004, ul-style)
Unordered list style
233-233: Expected: asterisk; Actual: dash (MD004, ul-style)
Unordered list style
234-234: Expected: asterisk; Actual: dash (MD004, ul-style)
Unordered list style
235-235: Expected: asterisk; Actual: dash (MD004, ul-style)
Unordered list style
236-236: Expected: asterisk; Actual: dash (MD004, ul-style)
Unordered list style
237-237: Expected: asterisk; Actual: dash (MD004, ul-style)
Unordered list style
238-238: Expected: asterisk; Actual: dash (MD004, ul-style)
Unordered list style
239-239: Expected: asterisk; Actual: dash (MD004, ul-style)
Unordered list style
240-240: Expected: asterisk; Actual: dash (MD004, ul-style)
Unordered list style
241-241: Expected: asterisk; Actual: dash (MD004, ul-style)
Unordered list style
242-242: Expected: asterisk; Actual: dash (MD004, ul-style)
Unordered list style
243-243: Expected: asterisk; Actual: dash (MD004, ul-style)
Unordered list style
244-244: Expected: asterisk; Actual: dash (MD004, ul-style)
Unordered list style
245-245: Expected: asterisk; Actual: dash (MD004, ul-style)
Unordered list style
246-246: Expected: asterisk; Actual: dash (MD004, ul-style)
Unordered list style
247-247: Expected: asterisk; Actual: dash (MD004, ul-style)
Unordered list style
248-248: Expected: asterisk; Actual: dash (MD004, ul-style)
Unordered list style
249-249: Expected: asterisk; Actual: dash (MD004, ul-style)
Unordered list style
250-250: Expected: asterisk; Actual: dash (MD004, ul-style)
Unordered list style
251-251: Expected: asterisk; Actual: dash (MD004, ul-style)
Unordered list style
252-252: Expected: asterisk; Actual: dash (MD004, ul-style)
Unordered list style
253-253: Expected: asterisk; Actual: dash (MD004, ul-style)
Unordered list style
254-254: Expected: asterisk; Actual: dash (MD004, ul-style)
Unordered list style
255-255: Expected: asterisk; Actual: dash (MD004, ul-style)
Unordered list style
256-256: Expected: asterisk; Actual: dash (MD004, ul-style)
Unordered list style
285-285: Expected: asterisk; Actual: dash (MD004, ul-style)
Unordered list style
286-286: Expected: asterisk; Actual: dash (MD004, ul-style)
Unordered list style
287-287: Expected: asterisk; Actual: dash (MD004, ul-style)
Unordered list style
364-364: Expected: asterisk; Actual: dash (MD004, ul-style)
Unordered list style
365-365: Expected: asterisk; Actual: dash (MD004, ul-style)
Unordered list style
366-366: Expected: asterisk; Actual: dash (MD004, ul-style)
Unordered list style
367-367: Expected: asterisk; Actual: dash (MD004, ul-style)
Unordered list style
368-368: Expected: asterisk; Actual: dash (MD004, ul-style)
Unordered list style
369-369: Expected: asterisk; Actual: dash (MD004, ul-style)
Unordered list style
370-370: Expected: asterisk; Actual: dash (MD004, ul-style)
Unordered list style
371-371: Expected: asterisk; Actual: dash (MD004, ul-style)
Unordered list style
372-372: Expected: asterisk; Actual: dash (MD004, ul-style)
Unordered list style
373-373: Expected: asterisk; Actual: dash (MD004, ul-style)
Unordered list style
374-374: Expected: asterisk; Actual: dash (MD004, ul-style)
Unordered list style
375-375: Expected: asterisk; Actual: dash (MD004, ul-style)
Unordered list style
376-376: Expected: asterisk; Actual: dash (MD004, ul-style)
Unordered list style
377-377: Expected: asterisk; Actual: dash (MD004, ul-style)
Unordered list style
378-378: Expected: asterisk; Actual: dash (MD004, ul-style)
Unordered list style
379-379: Expected: asterisk; Actual: dash (MD004, ul-style)
Unordered list style
380-380: Expected: asterisk; Actual: dash (MD004, ul-style)
Unordered list style
381-381: Expected: asterisk; Actual: dash (MD004, ul-style)
Unordered list style
382-382: Expected: asterisk; Actual: dash (MD004, ul-style)
Unordered list style
383-383: Expected: asterisk; Actual: dash (MD004, ul-style)
Unordered list style
384-384: Expected: asterisk; Actual: dash (MD004, ul-style)
Unordered list style
385-385: Expected: asterisk; Actual: dash (MD004, ul-style)
Unordered list style
386-386: Expected: asterisk; Actual: dash (MD004, ul-style)
Unordered list style
742-742: Expected: asterisk; Actual: dash (MD004, ul-style)
Unordered list style
750-750: Expected: asterisk; Actual: dash (MD004, ul-style)
Unordered list style
795-795: Expected: asterisk; Actual: dash (MD004, ul-style)
Unordered list style
803-803: Expected: asterisk; Actual: dash (MD004, ul-style)
Unordered list style
849-849: Expected: 1; Actual: 2 (MD012, no-multiple-blanks)
Multiple consecutive blank lines
1250-1250: Expected: 1; Actual: 2 (MD012, no-multiple-blanks)
Multiple consecutive blank lines
1267-1267: Expected: 1; Actual: 2 (MD012, no-multiple-blanks)
Multiple consecutive blank lines
1319-1319: Expected: 1; Actual: 2 (MD012, no-multiple-blanks)
Multiple consecutive blank lines
1423-1423: Expected: 1; Actual: 2 (MD012, no-multiple-blanks)
Multiple consecutive blank lines
1245-1245: null (MD019, no-multiple-space-atx)
Multiple spaces after hash on atx style heading
785-785: null (MD024, no-duplicate-heading)
Multiple headings with the same content
826-826: null (MD024, no-duplicate-heading)
Multiple headings with the same content
1120-1120: null (MD024, no-duplicate-heading)
Multiple headings with the same content
260-260: null (MD034, no-bare-urls)
Bare URL used
291-291: null (MD034, no-bare-urls)
Bare URL used
390-390: null (MD034, no-bare-urls)
Bare URL used
440-440: null (MD034, no-bare-urls)
Bare URL used
452-452: null (MD034, no-bare-urls)
Bare URL used
468-468: null (MD034, no-bare-urls)
Bare URL used
539-539: null (MD034, no-bare-urls)
Bare URL used
550-550: null (MD034, no-bare-urls)
Bare URL used
558-558: null (MD034, no-bare-urls)
Bare URL used
594-594: null (MD034, no-bare-urls)
Bare URL used
619-619: null (MD034, no-bare-urls)
Bare URL used
631-631: null (MD034, no-bare-urls)
Bare URL used
677-677: null (MD034, no-bare-urls)
Bare URL used
678-678: null (MD034, no-bare-urls)
Bare URL used
679-679: null (MD034, no-bare-urls)
Bare URL used
680-680: null (MD034, no-bare-urls)
Bare URL used
681-681: null (MD034, no-bare-urls)
Bare URL used
728-728: null (MD034, no-bare-urls)
Bare URL used
769-769: null (MD034, no-bare-urls)
Bare URL used
822-822: null (MD034, no-bare-urls)
Bare URL used
935-935: null (MD034, no-bare-urls)
Bare URL used
977-977: null (MD037, no-space-in-emphasis)
Spaces inside emphasis markers
977-977: null (MD037, no-space-in-emphasis)
Spaces inside emphasis markers
373-373: null (MD038, no-space-in-code)
Spaces inside code span elements
Additional comments not posted (7)
app/upgrades.go (6)
17-17: Added import forconsensusparamtypes. Ensure that this module is utilized appropriately in the upgrade handlers.
58-58: The addition of modules (crisistypes.ModuleName,circuittypes.ModuleName,consensusparamtypes.ModuleName) to the upgrade handlers is noted. Ensure that their respective migrations and configurations are handled correctly in the upgrade logic.Also applies to: 102-102
316-323: The migration of consensus parameters has been logged thoroughly. Ensure that all steps are well-documented and that error handling is robust, as previously discussed.
391-393: The migration logic for marker parameters uses the newsdkmath.Inttype formaxSupply. This change should ensure type safety and prevent potential issues with integer overflows. Good improvement.
405-406: The simplification in setting the default metadata OS locator parameters is a good clean-up. This should help in maintaining the code and reduce potential errors from misconfiguration.
475-475: The migration of IBC hooks parameters now correctly fetches theallowlistfrom the parameter store. This is crucial for maintaining the expected behavior of async acknowledgments in IBC transactions.CHANGELOG.md (1)
Line range hint
373-373: Please remove spaces inside code span elements to correct the Markdown formatting.
[ISSDUE]- ` this is not correct ` + `this is correct`
Description
This PR fixes the
umberandumber-rcupgrades that were failing for the following reasons:tp(instead ofpb). So the group migration failed when it tried to do stuff with existing addresses in state. This was becauseSetConfigcould change the HRP totp, but not back topb. At the beginning ofNewRootCmdwe were changing it totp. Later, in thePersistentPreRunE, without the--testnetflag, we were callingSetConfig(false, <seal>)which ultimately wasn't doing anything.panic: parameter uploadAccess not registered.This PR also moves the upgrade registration further down in
app.New. The call toinjectUpgradenow needs to be added in there. Part of that injection is providing a different pre-blocker (used to be begin-blocker). But the call toregisterUpgradeHandlerswas above the call toapp.SetPreBlockerso that customization was being undone, and I had to move it back down below.Related to:
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/)godoccomments.Unreleasedsection inCHANGELOG.mdFiles changedin the Github PR explorerCodecov Reportin the comment section below once CI passesSummary by CodeRabbit
New Features
Bug Fixes
Refactor
Chores