refactor: remove x/exp dep #21281
refactor: remove x/exp dep #21281
Conversation
WalkthroughWalkthroughThe recent changes primarily focus on optimizing the Go codebase by replacing the Changes
Assessment against linked issues
Possibly related issues
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 Configuration File (
|
github-actions
bot
added
C:x/staking
C:CLI
C:x/gov
C:Store
C:x/bank
C:x/auth
C:x/genutil
julienrbrt
added
the
backport/v0.52.x
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
Outside diff range, codebase verification and nitpick comments (1)
scripts/dep-assert.sh (1)
Dependencies on
golang.org/x/expFound ingo.modFilesThe verification indicates that several
go.modfiles in thex/directory still have dependencies ongolang.org/x/exp. This confirms that the check forx/expdependencies is effective and identifies these dependencies accurately.
- Files with
golang.org/x/expdependency:
x/accounts/defaults/lockup/go.modx/accounts/defaults/multisig/go.modx/accounts/go.modx/auth/go.modx/authz/go.modx/bank/go.modx/circuit/go.modx/consensus/go.modx/distribution/go.modx/epochs/go.modx/evidence/go.modx/feegrant/go.modx/gov/go.modx/group/go.modx/mint/go.modx/nft/go.modx/params/go.modx/protocolpool/go.modx/slashing/go.modx/staking/go.modx/tx/go.modx/upgrade/go.modPlease review these dependencies to assess if they need to be updated or removed.
Analysis chain
Line range hint
25-42:
Verify the effectiveness of thex/expdependency check.The script now checks for
golang.org/x/expdependencies in thex/directory. Ensure that this check correctly identifies any remaining dependencies onx/exp.Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify that the script correctly identifies dependencies on `golang.org/x/exp`. # Test: Run the script and check for any output indicating `x/exp` dependencies. Expect: No output. ./scripts/dep-assert.shLength of output: 6353
Script:
#!/bin/bash # Find all go.mod files in the x/ directory and check for dependencies on golang.org/x/exp fd --type f --name 'go.mod' x/ --exec grep -H 'golang.org/x/exp' {}Length of output: 318
Script:
#!/bin/bash # Find all go.mod files in the x/ directory and check for dependencies on golang.org/x/exp fd 'go.mod' x/ | xargs grep -H 'golang.org/x/exp'Length of output: 1875
Review details
Configuration used: .coderabbit.yml
Review profile: CHILL
Files selected for processing (36)
- baseapp/baseapp.go (3 hunks)
- go.mod (2 hunks)
- runtime/v2/app.go (1 hunks)
- runtime/v2/go.mod (2 hunks)
- runtime/v2/manager.go (2 hunks)
- scripts/dep-assert.sh (2 hunks)
- server/v2/api/grpc/server.go (2 hunks)
- server/v2/go.mod (2 hunks)
- server/v2/stf/core_event_service.go (3 hunks)
- server/v2/stf/go.mod (1 hunks)
- store/v2/commitment/store.go (2 hunks)
- store/v2/go.mod (2 hunks)
- tests/systemtests/cli.go (1 hunks)
- tests/systemtests/go.mod (2 hunks)
- tests/systemtests/system.go (3 hunks)
- tools/confix/cmd/diff.go (2 hunks)
- tools/confix/cmd/migrate.go (2 hunks)
- tools/confix/go.mod (2 hunks)
- types/events.go (2 hunks)
- types/module/module.go (2 hunks)
- x/auth/ante/unorderedtx/manager.go (2 hunks)
- x/auth/go.mod (2 hunks)
- x/bank/depinject.go (2 hunks)
- x/bank/go.mod (1 hunks)
- x/epochs/depinject.go (2 hunks)
- x/epochs/go.mod (1 hunks)
- x/epochs/keeper/abci_test.go (2 hunks)
- x/feegrant/module/depinject.go (2 hunks)
- x/feegrant/module/module.go (2 hunks)
- x/genutil/client/cli/migrate.go (2 hunks)
- x/gov/depinject.go (2 hunks)
- x/gov/go.mod (2 hunks)
- x/group/go.mod (2 hunks)
- x/group/keeper/invariants.go (2 hunks)
- x/staking/depinject.go (3 hunks)
- x/staking/go.mod (2 hunks)
Files skipped from review due to trivial changes (17)
- go.mod
- runtime/v2/app.go
- runtime/v2/go.mod
- server/v2/go.mod
- server/v2/stf/go.mod
- store/v2/go.mod
- tests/systemtests/cli.go
- tests/systemtests/go.mod
- tests/systemtests/system.go
- tools/confix/go.mod
- types/events.go
- x/auth/go.mod
- x/bank/go.mod
- x/epochs/go.mod
- x/gov/go.mod
- x/group/go.mod
- x/staking/go.mod
Additional context used
Path-based instructions (18)
x/feegrant/module/depinject.go (1)
Pattern
**/*.go: Review the Golang code for conformity with the Uber Golang style guide, highlighting any deviations.x/epochs/depinject.go (1)
Pattern
**/*.go: Review the Golang code for conformity with the Uber Golang style guide, highlighting any deviations.server/v2/stf/core_event_service.go (1)
Pattern
**/*.go: Review the Golang code for conformity with the Uber Golang style guide, highlighting any deviations.tools/confix/cmd/diff.go (1)
Pattern
**/*.go: Review the Golang code for conformity with the Uber Golang style guide, highlighting any deviations.tools/confix/cmd/migrate.go (1)
Pattern
**/*.go: Review the Golang code for conformity with the Uber Golang style guide, highlighting any deviations.x/bank/depinject.go (1)
Pattern
**/*.go: Review the Golang code for conformity with the Uber Golang style guide, highlighting any deviations.x/gov/depinject.go (1)
Pattern
**/*.go: Review the Golang code for conformity with the Uber Golang style guide, highlighting any deviations.x/group/keeper/invariants.go (1)
Pattern
**/*.go: Review the Golang code for conformity with the Uber Golang style guide, highlighting any deviations.x/staking/depinject.go (1)
Pattern
**/*.go: Review the Golang code for conformity with the Uber Golang style guide, highlighting any deviations.x/genutil/client/cli/migrate.go (1)
Pattern
**/*.go: Review the Golang code for conformity with the Uber Golang style guide, highlighting any deviations.server/v2/api/grpc/server.go (1)
Pattern
**/*.go: Review the Golang code for conformity with the Uber Golang style guide, highlighting any deviations.x/feegrant/module/module.go (1)
Pattern
**/*.go: Review the Golang code for conformity with the Uber Golang style guide, highlighting any deviations.x/auth/ante/unorderedtx/manager.go (1)
Pattern
**/*.go: Review the Golang code for conformity with the Uber Golang style guide, highlighting any deviations.x/epochs/keeper/abci_test.go (2)
Pattern
**/*.go: Review the Golang code for conformity with the Uber Golang style guide, highlighting any deviations.
Pattern
**/*_test.go: "Assess the unit test code assessing sufficient code coverage for the changes associated in the pull request"store/v2/commitment/store.go (1)
Pattern
**/*.go: Review the Golang code for conformity with the Uber Golang style guide, highlighting any deviations.runtime/v2/manager.go (1)
Pattern
**/*.go: Review the Golang code for conformity with the Uber Golang style guide, highlighting any deviations.types/module/module.go (1)
Pattern
**/*.go: Review the Golang code for conformity with the Uber Golang style guide, highlighting any deviations.baseapp/baseapp.go (1)
Pattern
**/*.go: Review the Golang code for conformity with the Uber Golang style guide, highlighting any deviations.
Additional comments not posted (23)
x/feegrant/module/depinject.go (1)
Line range hint
1-1:
Verify the impact of removing simulation-related functions.The removal of
GenerateGenesisState,RegisterStoreDecoder, andWeightedOperationsfrom theAppModulemight affect simulation scenarios. Ensure that these functionalities are no longer required or have been implemented elsewhere.Verification successful
No impact from removing simulation-related functions in
x/feegrantmodule.The functions
GenerateGenesisState,RegisterStoreDecoder, andWeightedOperationswere removed from thex/feegrant/module/depinject.gofile. There are no other references to these functions within thex/feegrantmodule, indicating that their removal does not impact the module's functionality. Other modules still contain these functions, suggesting that their simulation-related functionalities are module-specific.Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Check for references to removed simulation-related functions in the codebase. # Test: Search for references to the removed functions. Expect: No references found. rg --type go $'GenerateGenesisState|RegisterStoreDecoder|WeightedOperations'Length of output: 19707
x/epochs/depinject.go (1)
5-6: Verify the correctness of the new sorting implementation.The use of
slices.Sorted(maps.Keys(hooks))simplifies the sorting process. Ensure that this change maintains the intended functionality of sorting module names lexically.Also applies to: 58-60
server/v2/stf/core_event_service.go (2)
7-7: Use of standard librarymapsis appropriate.The import of the standard library
mapsis a good change, aligning with the goal of removing dependencies on experimental packages.
67-67: Efficient key sorting withslices.Sorted(maps.Keys(attrMap)).The use of
slices.Sorted(maps.Keys(attrMap))efficiently combines key retrieval and sorting, enhancing both readability and performance.tools/confix/cmd/diff.go (1)
47-47: Improved key handling withslices.Collect(maps.Keys(confix.Migrations)).The change to use
slices.Collect(maps.Keys(confix.Migrations))simplifies the collection and formatting of map keys, enhancing clarity and efficiency in error messages.tools/confix/cmd/migrate.go (1)
64-64: Consistent and efficient key handling withslices.Collect(maps.Keys(confix.Migrations)).Using
slices.Collect(maps.Keys(confix.Migrations))maintains consistency across the codebase and improves the handling of map keys in error messages.x/bank/depinject.go (1)
106-106: Verify the use ofslices.Collectfor map keys.The use of
slices.Collect(maps.Keys(restrictions))is a functional approach to collect keys from the map. Ensure that this does not introduce unnecessary overhead or alter the intended logic.x/gov/depinject.go (1)
123-123: Verify the use ofslices.Sortedfor sorting module names.The use of
slices.Sorted(maps.Keys(govHooks))simplifies the sorting process. Ensure that this change maintains the intended order and does not introduce inefficiencies.Verification successful
Use of
slices.Sortedis appropriate and consistent.The use of
slices.Sorted(maps.Keys(govHooks))inInvokeSetHooksis consistent across the codebase and aligns with the intended lexical ordering of module names. This change simplifies the sorting process without introducing inefficiencies. No further action is needed.Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify that the use of `slices.Sorted` correctly sorts module names and maintains the intended order. # Test: Search for the function usage. Expect: Correct sorting of module names without performance degradation. rg --type go -A 5 $'InvokeSetHooks'Length of output: 1129
Script:
#!/bin/bash # Search for the usage of `slices.Sorted` in the repository to verify its implementation. rg --type go 'slices.Sorted'Length of output: 882
x/group/keeper/invariants.go (1)
60-67: Verify the custom sorting logic withslices.SortFunc.The use of
slices.SortFuncwith a custom comparator for sorting group IDs is explicit and clear. Ensure that the comparator is correctly implemented and efficient.Verification successful
Custom sorting logic is correctly implemented and efficient.
The custom comparator used in
slices.SortFuncfor sorting group IDs is correctly implemented. It efficiently sorts the IDs by comparing them directly and returning appropriate values for sorting. No issues were found with the logic or efficiency of this implementation.Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify that the custom sorting logic implemented using `slices.SortFunc` is correct and efficient. # Test: Check for the implementation of the custom comparator. Expect: Correct and efficient sorting of group IDs. rg --type go -A 5 $'GroupTotalWeightInvariantHelper'Length of output: 1178
Script:
#!/bin/bash # Extract the full implementation of the GroupTotalWeightInvariantHelper function to verify the sorting logic. ast-grep --lang go --pattern 'func GroupTotalWeightInvariantHelper($_, $_, $_, $_) { $$$ }'Length of output: 93
Script:
#!/bin/bash # Use rg to search for the GroupTotalWeightInvariantHelper function definition and include additional lines for full context. rg --type go -A 50 'func GroupTotalWeightInvariantHelper'Length of output: 3082
x/staking/depinject.go (2)
95-97: Efficient early return for empty hooks.The addition of an early return when
stakingHooksis empty improves efficiency by avoiding unnecessary processing.
99-99: Use ofslices.Collectfor key collection.The transition to
slices.Collect(maps.Keys(stakingHooks))is a good step towards using standard library features, aligning with the PR objectives to removex/expdependencies.x/genutil/client/cli/migrate.go (1)
55-55: Simplified sorting withslices.Sorted.The use of
slices.Sortedto sort version keys is a concise and efficient replacement for traditional sorting methods, enhancing readability.server/v2/api/grpc/server.go (1)
69-69: Improved key collection withslices.Collect.The update to use
slices.Collect(maps.Keys(methodsMap))enhances the processing of method keys for gRPC reflection, aligning with the removal ofx/expdependencies.x/feegrant/module/module.go (3)
160-163: LGTM!The
GenerateGenesisStatemethod correctly utilizessimulation.RandomizedGenStateto create a randomized genesis state.
165-168: LGTM!The
RegisterStoreDecodermethod correctly registers a store decoder for the feegrant module's types.
170-175: LGTM!The
WeightedOperationsmethod accurately aggregates operations with their respective weights for the feegrant module.x/auth/ante/unorderedtx/manager.go (1)
164-164: LGTM!The use of
slices.SortedFuncsimplifies the sorting logic and is a suitable replacement forsort.Slice.x/epochs/keeper/abci_test.go (1)
92-99: LGTM!The updated sorting logic in
TestEpochInfoBeginBlockChangescorrectly handles ordering based on timestamps, ensuring robust test scenarios.store/v2/commitment/store.go (1)
114-114: Good use ofslices.Sortedfor sorting keys.The use of
slices.Sorted(maps.Keys(c.multiTrees))improves readability and conciseness by combining key extraction and sorting into a single step. This change aligns with modern Go idioms.runtime/v2/manager.go (1)
45-45: Effective use ofslices.Collectfor key collection.The use of
slices.Collect(maps.Keys(modules))efficiently handles the collection of module keys, potentially improving performance and readability.types/module/module.go (1)
836-836: Enhanced flexibility withslices.Collectfor module names.The use of
slices.Collect(maps.Keys(m.Modules))allows for additional operations on the returned slice, enhancing flexibility and potential performance.baseapp/baseapp.go (2)
7-9: Imports update approved.The change to use
mapsandslicesfrom the standard library instead ofx/expis appropriate and aligns with the PR objectives.
343-346: Optimized sorting withslices.Sortedapproved.The use of
slices.Sorted(maps.Keys(keys))streamlines the sorting process, enhancing performance by reducing function calls.Ensure that the usage of
slices.Sortedis consistent with the expected behavior and that it correctly replaces the previous sorting mechanism.Verification successful
Consistent use of
slices.Sortedverified across the codebase.The implementation of
slices.Sortedinbaseapp/baseapp.gois consistent with its usage in other parts of the codebase, confirming the adoption of this function for sorting operations. This change appears to be part of a broader optimization effort.
baseapp/baseapp.go: Usage ofslices.Sorted(maps.Keys(keys))aligns with the pattern across the codebase.Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify the usage of `slices.Sorted` in the codebase. # Test: Search for the usage of `slices.Sorted`. Expect: Correct usage replacing previous sorting mechanisms. rg --type go 'slices.Sorted'Length of output: 882
There was a problem hiding this comment.
Actionable comments posted: 0
Review details
Configuration used: .coderabbit.yml
Review profile: CHILL
Files selected for processing (1)
- tests/systemtests/system.go (4 hunks)
Files skipped from review as they are similar to previous changes (1)
- tests/systemtests/system.go
There was a problem hiding this comment.
Actionable comments posted: 0
Review details
Configuration used: .coderabbit.yml
Review profile: CHILL
Files selected for processing (2)
- runtime/v2/app.go (1 hunks)
- runtime/v2/manager.go (2 hunks)
Files skipped from review due to trivial changes (1)
- runtime/v2/manager.go
Files skipped from review as they are similar to previous changes (1)
- runtime/v2/app.go
There was a problem hiding this comment.
Actionable comments posted: 0
Review details
Configuration used: .coderabbit.yml
Review profile: CHILL
Files selected for processing (1)
- runtime/v2/manager.go (2 hunks)
Files skipped from review as they are similar to previous changes (1)
- runtime/v2/manager.go
There was a problem hiding this comment.
Actionable comments posted: 0
Review details
Configuration used: .coderabbit.yml
Review profile: CHILL
Files ignored due to path filters (1)
store/v2/go.sumis excluded by!**/*.sum
Files selected for processing (12)
- go.mod (2 hunks)
- runtime/v2/go.mod (2 hunks)
- server/v2/go.mod (2 hunks)
- store/v2/go.mod (2 hunks)
- tests/systemtests/go.mod (2 hunks)
- tools/confix/go.mod (2 hunks)
- x/auth/go.mod (2 hunks)
- x/bank/go.mod (1 hunks)
- x/epochs/go.mod (1 hunks)
- x/gov/go.mod (2 hunks)
- x/group/go.mod (2 hunks)
- x/staking/go.mod (2 hunks)
Files skipped from review due to trivial changes (3)
- go.mod
- runtime/v2/go.mod
- x/staking/go.mod
Files skipped from review as they are similar to previous changes (9)
- server/v2/go.mod
- store/v2/go.mod
- tests/systemtests/go.mod
- tools/confix/go.mod
- x/auth/go.mod
- x/bank/go.mod
- x/epochs/go.mod
- x/gov/go.mod
- x/group/go.mod
(cherry picked from commit 6f30de3) # Conflicts: # runtime/v2/app.go # runtime/v2/go.mod # runtime/v2/manager.go # server/v2/api/grpc/server.go # server/v2/go.mod # server/v2/stf/core_event_service.go # server/v2/stf/go.mod # store/v2/commitment/store.go # store/v2/go.mod # store/v2/go.sum # tests/systemtests/go.mod # tools/confix/go.mod # x/staking/go.mod
Description
Closes: #19892
Finally! Came to hate that package because it has bitten us.
We can finally break freeeee.
Blocked on #21280
Author Checklist
All items are required. Please add a note to the item if the item is not applicable and
please add links to any relevant follow up issues.
I have...
!in the type prefix if API or client breaking changeCHANGELOG.mdReviewers Checklist
All items are required. Please add a note if the item is not applicable and please add
your handle next to the items reviewed if you only reviewed selected items.
Please see Pull Request Reviewer section in the contributing guide for more information on how to review a pull request.
I have...
Summary by CodeRabbit
New Features
slicespackage for improved performance and clarity.Bug Fixes
Refactor
NewModuleManagerfunction to sort module keys prior to further operations.Style
slicespackage instead of external dependencies, enhancing clarity in the codebase.