refactor: Modernize sorting #1947

joe-bowman · 2025-07-28T10:17:42Z

1. Summary

Fixes #1946

2.Type of change

Performance improvement - non-functional and non-breaking change that improves efficiency.

3. Implementation details

Migrate sort.Slice and sort.StableSlice to new slices.SortFunc and slices.StableSortFunc introduced in go1.22.

Summary by CodeRabbit

Refactor
- Updated sorting logic across multiple areas to use the Go standard library's new slices package for more explicit and stable sorting behavior.
- Replaced boolean comparators with three-way integer comparators for improved clarity and maintainability.
- Added explicit default cases in several switch statements to clarify control flow, with no impact on existing logic.
Tests
- Updated test sorting logic to ensure deterministic ordering using the new slices package.

Replaced sort.Slice and sort.SliceStable with the more modern and performant slices.Sort and slices.SortStable. Also fixed some unrelated linting errors.

coderabbitai · 2025-07-28T10:17:49Z

Walkthrough

The changes systematically replace usages of sort.Slice and sort.SliceStable with slices.SortFunc and slices.SortStableFunc throughout the codebase, updating comparator functions as required. Additionally, several switch statements are modified to include explicit default: cases, enhancing code clarity without altering logic.

Changes

Cohort / File(s)	Change Summary
Switch Statement Default Cases `x/interchainstaking/keeper/callbacks.go`, `x/interchainstaking/keeper/fuzz_test.go`, `x/interchainstaking/keeper/keeper.go`, `x/interchainstaking/types/rebalance.go`	Added explicit `default:` cases (with no code) to various `switch` statements, clarifying control flow without changing behavior.
Sorting Migration: Interchainstaking Types `x/interchainstaking/types/delegation.go`, `x/interchainstaking/types/rebalance.go`	Replaced `sort.SliceStable` with `slices.SortStableFunc` for stable sorting; updated comparator functions to return integer values as required by the new API.
Sorting Migration: Interchainstaking Keeper Tests `x/interchainstaking/keeper/zones_test.go`	Migrated from `sort.Slice` to `slices.SortFunc` and updated the comparator to a three-way integer return value for sorting `Zone` slices by `ChainId`.
Sorting Migration: Supply Keeper `x/supply/keeper/keeper.go`	Replaced `sort.Slice` with `slices.SortFunc` in the `TopN` function, updating comparator to integer-based as per new API, for sorting accounts by balance in descending order.

Sequence Diagram(s)

sequenceDiagram
    participant Caller
    participant Keeper
    participant SlicesPkg

    Caller->>Keeper: Call sorting function (e.g., ValidatorIntents.Sort)
    Keeper->>SlicesPkg: Use slices.SortFunc/SortStableFunc with integer comparator
    SlicesPkg-->>Keeper: Returns sorted slice
    Keeper-->>Caller: Returns result

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~8 minutes

Assessment against linked issues

Objective	Addressed	Explanation
Migrate all uses of sort.Slice and sort.SliceStable to slices.SortFunc and slices.SortStableFunc (#1946)	✅

Poem

A hop and a skip, through slices we leap,
Old sort now replaced, our code’s trim and neat.
With comparators clear, and defaults in place,
The rabbits rejoice—our code’s in good grace!
🐇✨

“No more reflection,” the bunnies all cheer,
For faster and cleaner, the future is here!

Note

⚡️ Unit Test Generation is now available in beta!

Learn more here, or try it out under "Finishing Touches" below.

📜 Recent review details

Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between e1b5e68 and 16cfa0e.

⛔ Files ignored due to path filters (1)

go.work.sum is excluded by !**/*.sum

📒 Files selected for processing (7)

x/interchainstaking/keeper/callbacks.go (1 hunks)
x/interchainstaking/keeper/fuzz_test.go (1 hunks)
x/interchainstaking/keeper/keeper.go (1 hunks)
x/interchainstaking/keeper/zones_test.go (2 hunks)
x/interchainstaking/types/delegation.go (2 hunks)
x/interchainstaking/types/rebalance.go (4 hunks)
x/supply/keeper/keeper.go (2 hunks)

🧰 Additional context used

🧠 Learnings (5)

x/interchainstaking/keeper/callbacks.go (7)

Learnt from: joe-bowman
PR: #1132
File: x/interchainstaking/keeper/callbacks.go:662-664
Timestamp: 2024-07-29T07:24:15.377Z
Learning: The DecrementWithdrawalWaitgroup method in x/interchainstaking/keeper/callbacks.go includes underflow protection, ensuring safe decrement operations.

Learnt from: joe-bowman
PR: #1132
File: x/interchainstaking/keeper/callbacks.go:127-127
Timestamp: 2024-10-15T13:03:53.675Z
Learning: The DecrementWithdrawalWaitgroup method in x/interchainstaking/keeper/callbacks.go internally handles underflow protection, ensuring that the waitgroup count does not become negative.

Learnt from: joe-bowman
PR: #1132
File: x/interchainstaking/keeper/callbacks.go:625-625
Timestamp: 2024-07-29T07:24:15.377Z
Learning: The DecrementWithdrawalWaitgroup method in x/interchainstaking/keeper/callbacks.go internally handles underflow protection, ensuring that the waitgroup count does not become negative.

Learnt from: joe-bowman
PR: #1132
File: x/interchainstaking/keeper/callbacks.go:625-625
Timestamp: 2024-10-15T13:03:53.675Z
Learning: The DecrementWithdrawalWaitgroup method in x/interchainstaking/keeper/callbacks.go internally handles underflow protection, ensuring that the waitgroup count does not become negative.

Learnt from: joe-bowman
PR: #1132
File: x/interchainstaking/keeper/callbacks.go:127-127
Timestamp: 2024-07-29T07:24:15.377Z
Learning: The DecrementWithdrawalWaitgroup method in x/interchainstaking/keeper/callbacks.go internally handles underflow protection, ensuring that the waitgroup count does not become negative.

Learnt from: arhamchordia
PR: #1803
File: x/supply/keeper/keeper_test.go:31-66
Timestamp: 2025-01-28T19:03:57.668Z
Learning: Avoid suggesting test cases that duplicate functionality already tested in msg_server_test.go, particularly for module account operations and balance transfers. Focus on unique keeper functionality instead.

Learnt from: joe-bowman
PR: #1767
File: app/upgrades/v1_7.go:141-161
Timestamp: 2024-12-03T11:18:49.168Z
Learning: In the V010705UpgradeHandler function in app/upgrades/v1_7.go, the OverrideRedemptionRateNoCap method computes and sets zone.RedemptionRate at runtime, making it unnecessary to set it explicitly.

x/interchainstaking/keeper/keeper.go (3)

Learnt from: joe-bowman
PR: #1767
File: app/upgrades/v1_7.go:141-161
Timestamp: 2024-12-03T11:18:49.168Z
Learning: In the V010705UpgradeHandler function in app/upgrades/v1_7.go, the OverrideRedemptionRateNoCap method computes and sets zone.RedemptionRate at runtime, making it unnecessary to set it explicitly.

Learnt from: arhamchordia
PR: #1803
File: x/supply/keeper/keeper_test.go:31-66
Timestamp: 2025-01-28T19:03:57.668Z
Learning: Avoid suggesting test cases that duplicate functionality already tested in msg_server_test.go, particularly for module account operations and balance transfers. Focus on unique keeper functionality instead.

Learnt from: joe-bowman
PR: #1132
File: x/interchainstaking/keeper/callbacks.go:662-664
Timestamp: 2024-07-29T07:24:15.377Z
Learning: The DecrementWithdrawalWaitgroup method in x/interchainstaking/keeper/callbacks.go includes underflow protection, ensuring safe decrement operations.

x/interchainstaking/keeper/fuzz_test.go (1)

Learnt from: arhamchordia
PR: #1803
File: x/supply/keeper/keeper_test.go:31-66
Timestamp: 2025-01-28T19:03:57.668Z
Learning: Avoid suggesting test cases that duplicate functionality already tested in msg_server_test.go, particularly for module account operations and balance transfers. Focus on unique keeper functionality instead.

x/supply/keeper/keeper.go (2)

Learnt from: arhamchordia
PR: #1803
File: x/supply/keeper/keeper_test.go:31-66
Timestamp: 2025-01-28T19:03:57.668Z
Learning: Avoid suggesting test cases that duplicate functionality already tested in msg_server_test.go, particularly for module account operations and balance transfers. Focus on unique keeper functionality instead.

Learnt from: joe-bowman
PR: #1800
File: x/supply/keeper/grpc_query.go:48-58
Timestamp: 2025-01-27T17:21:14.046Z
Learning: The TopN endpoint in the supply module uses uint64 throughout to maintain type consistency and prevent integer overflow issues.

x/interchainstaking/keeper/zones_test.go (1)

Learnt from: arhamchordia
PR: #1803
File: x/supply/keeper/keeper_test.go:31-66
Timestamp: 2025-01-28T19:03:57.668Z
Learning: Avoid suggesting test cases that duplicate functionality already tested in msg_server_test.go, particularly for module account operations and balance transfers. Focus on unique keeper functionality instead.

⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (5)

GitHub Check: Analyze
GitHub Check: test quicksilver (darwin, arm64)
GitHub Check: test quicksilver (amd64, linux)
GitHub Check: test quicksilver (amd64, windows)
GitHub Check: lint

🔇 Additional comments (16)

x/interchainstaking/keeper/callbacks.go (1)

802-803: LGTM! Empty default case improves switch statement exhaustiveness.

The addition of an empty default: case makes the switch statement explicit about handling all possible cases. This improves code clarity without changing the existing logic.

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

304-305: LGTM! Empty default case enhances switch statement clarity.

Adding the empty default: case makes the switch statement exhaustive and explicitly handles all validator state comparisons. This aligns well with the existing logic flow.

x/interchainstaking/keeper/fuzz_test.go (1)

72-73: LGTM! Default case improves fuzzing test clarity.

The empty default: case ensures that fuzzing proceeds normally for all inputs except the known problematic vectors. This makes the control flow more explicit and maintains the intended test behavior.

x/supply/keeper/keeper.go (2)

6-6: LGTM! Successfully migrated import from sort to slices package.

This import change supports the modernization of sorting operations as outlined in the PR objectives.

131-138: LGTM! Correct migration from sort.Slice to slices.SortFunc.

The three-way comparator function correctly implements descending order sorting by balance:

Returns -1 when first account has higher balance (comes first)

Returns 1 when first account has lower balance (comes second)

Returns 0 when balances are equal

This change successfully modernizes the sorting implementation while maintaining the same behavior.

x/interchainstaking/keeper/zones_test.go (2)

6-6: LGTM! Import updated to support slices package usage.

This import change enables the use of the modernized sorting functions in the test file.

131-146: LGTM! Correct migration to slices.SortFunc for test determinism.

Both sorting operations correctly implement ascending order by ChainId using three-way comparators:

Returns -1 for ascending order when first ChainId is lexicographically smaller

Returns 1 when first ChainId is lexicographically larger

Returns 0 when ChainIds are equal

This maintains deterministic test behavior while modernizing the sorting implementation as intended by the PR.

x/interchainstaking/types/delegation.go (2)

5-5: LGTM: Import modernization

The import change from "sort" to "slices" aligns with the Go 1.22+ modernization objective to use the more efficient slices package.

72-79: LGTM: Correct migration to slices.SortStableFunc

The migration from sort.SliceStable to slices.SortStableFunc is correctly implemented:

The three-way comparator properly implements lexicographic string comparison

Returns -1, 0, or 1 as expected for the new API

Maintains the same stable sorting behavior by ValoperAddress in ascending order

Eliminates reflection overhead for improved performance

x/interchainstaking/types/rebalance.go (7)

6-6: LGTM: Import addition for slices package

Adding the "slices" import is correct for the modernization. The existing "sort" import is appropriately retained since sort.Strings is still used on line 41.

106-113: LGTM: Correct migration for ValoperAddress sorting

The migration from sort.SliceStable to slices.SortStableFunc with three-way string comparison is correctly implemented and maintains the same sorting behavior.

116-123: LGTM: Correct migration for Amount sorting

The three-way comparator correctly implements descending order sorting by Amount using the GT/LT methods, maintaining the same behavior as the original boolean comparator.

193-200: LGTM: Correct migration for Source sorting

The three-way string comparison for sorting by Source field is correctly implemented and maintains ascending order.

203-210: LGTM: Correct migration for Target sorting

The three-way string comparison for sorting by Target field is correctly implemented and maintains ascending order.

213-220: LGTM: Correct migration for Amount sorting

The three-way comparator correctly implements ascending order sorting by Amount using the LT/GT methods, maintaining the same behavior as the original boolean comparator.

298-299: LGTM: Good practice adding explicit default case

Adding an explicit empty default: case improves code clarity and makes the switch statement exhaustive, which is a good practice for maintainability.

✨ Finishing Touches

📝 Generate Docstrings

🧪 Generate unit tests

Create PR with unit tests
Post copyable unit tests in a comment
Commit unit tests in branch feature/modernize-sort

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

🪧 Tips

Chat

There are 3 ways to chat with CodeRabbit:

Review comments: Directly reply to a review comment made by CodeRabbit. Example:
- I pushed a fix in commit <commit_id>, please review it.
- Explain this complex logic.
- Open a follow-up GitHub issue for this discussion.
Files and specific lines of code (under the "Files changed" tab): Tag @coderabbitai in a new review comment at the desired location with your query. Examples:
- @coderabbitai explain this code block.
- @coderabbitai modularize this function.
PR comments: Tag @coderabbitai in a new PR comment to ask questions about the PR branch. For the best results, please provide a very specific query, as very limited context is provided in this mode. Examples:
- @coderabbitai gather interesting stats about this repository and render them as a table. Additionally, render a pie chart showing the language distribution in the codebase.
- @coderabbitai read src/utils.ts and explain its main purpose.
- @coderabbitai read the files in the src/scheduler package and generate a class diagram using mermaid and a README in the markdown format.
- @coderabbitai help me debug CodeRabbit configuration file.

Support

Need help? Create a ticket on our support page for assistance with any issues or questions.

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 using PR comments)

@coderabbitai pause to pause the reviews on a PR.
@coderabbitai resume to resume the paused reviews.
@coderabbitai review to trigger an incremental review. This is useful when automatic reviews are disabled for the repository.
@coderabbitai full review to do a full review from scratch and review all the files again.
@coderabbitai summary to regenerate the summary of the PR.
@coderabbitai generate docstrings to generate docstrings for this PR.
@coderabbitai generate sequence diagram to generate a sequence diagram of the changes in this PR.
@coderabbitai generate unit tests to generate unit tests for this PR.
@coderabbitai resolve resolve all the CodeRabbit review comments.
@coderabbitai configuration to show the current CodeRabbit configuration for the repository.
@coderabbitai help to get help.

Other keywords and placeholders

Add @coderabbitai ignore anywhere in the PR description to prevent this PR from being reviewed.
Add @coderabbitai summary to generate the high-level summary at a specific location in the PR description.
Add @coderabbitai anywhere in the PR title to generate the title automatically.

CodeRabbit Configuration File (`.coderabbit.yaml`)

You can programmatically configure CodeRabbit by adding a .coderabbit.yaml file to the root of your repository.
Please see the configuration documentation for more information.
If your editor has YAML language server enabled, you can add the path at the top of this file to enable auto-completion and validation: # yaml-language-server: $schema=https://coderabbit.ai/integrations/schema.v2.json

Documentation and Community

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

codecov · 2025-07-28T10:37:48Z

Codecov Report

❌ Patch coverage is 75.00000% with 13 lines in your changes missing coverage. Please review.
✅ Project coverage is 57.62%. Comparing base (e1b5e68) to head (16cfa0e).
⚠️ Report is 3 commits behind head on main.

Files with missing lines	Patch %	Lines
x/interchainstaking/types/rebalance.go	69.44%	6 Missing and 5 partials ⚠️
x/interchainstaking/keeper/callbacks.go	0.00%	1 Missing ⚠️
x/interchainstaking/types/delegation.go	85.71%	1 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #1947      +/-   ##
==========================================
- Coverage   57.63%   57.62%   -0.01%     
==========================================
  Files         172      172              
  Lines       15268    15296      +28     
==========================================
+ Hits         8800     8815      +15     
- Misses       5702     5710       +8     
- Partials      766      771       +5

Flag	Coverage Δ
unittests	`57.62% <75.00%> (-0.01%)`	⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

Files with missing lines	Coverage Δ
x/interchainstaking/keeper/keeper.go	`73.58% <100.00%> (+0.04%)`	⬆️
x/supply/keeper/keeper.go	`86.30% <100.00%> (+0.79%)`	⬆️
x/interchainstaking/keeper/callbacks.go	`60.54% <0.00%> (-0.10%)`	⬇️
x/interchainstaking/types/delegation.go	`75.47% <85.71%> (-0.02%)`	⬇️
x/interchainstaking/types/rebalance.go	`72.95% <69.44%> (-3.45%)`	⬇️

🚀 New features to boost your workflow:

❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

refactor: Modernize sorting

16cfa0e

Replaced sort.Slice and sort.SliceStable with the more modern and performant slices.Sort and slices.SortStable. Also fixed some unrelated linting errors.

joe-bowman requested review from ajansari95 and faddat as code owners July 28, 2025 10:17

ajansari95 approved these changes Aug 13, 2025

View reviewed changes

ajansari95 merged commit cdc71fc into main Aug 13, 2025
18 checks passed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

refactor: Modernize sorting #1947

refactor: Modernize sorting #1947

Uh oh!

joe-bowman commented Jul 28, 2025 •

edited by coderabbitai bot

Loading

Uh oh!

coderabbitai bot commented Jul 28, 2025 •

edited

Loading

Chat

Support

CodeRabbit Commands (Invoked using PR comments)

Other keywords and placeholders

CodeRabbit Configuration File (`.coderabbit.yaml`)

Documentation and Community

Uh oh!

codecov bot commented Jul 28, 2025 •

edited

Loading

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

refactor: Modernize sorting #1947

refactor: Modernize sorting #1947

Uh oh!

Conversation

joe-bowman commented Jul 28, 2025 • edited by coderabbitai bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

1. Summary

2.Type of change

3. Implementation details

Summary by CodeRabbit

Uh oh!

coderabbitai bot commented Jul 28, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Walkthrough

Changes

Sequence Diagram(s)

Estimated code review effort

Assessment against linked issues

Poem

Chat

Support

CodeRabbit Commands (Invoked using PR comments)

Other keywords and placeholders

CodeRabbit Configuration File (.coderabbit.yaml)

Documentation and Community

Uh oh!

codecov bot commented Jul 28, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

joe-bowman commented Jul 28, 2025 •

edited by coderabbitai bot

Loading

coderabbitai bot commented Jul 28, 2025 •

edited

Loading

CodeRabbit Configuration File (`.coderabbit.yaml`)

codecov bot commented Jul 28, 2025 •

edited

Loading