Skip to content

Conversation

beer-1
Copy link
Member

@beer-1 beer-1 commented Sep 16, 2025

Description

We decided not to proceed with the delegation migration flow and are removing its implementation.


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

  • included the correct type prefix in the PR title, you can find examples of the prefixes below:
  • confirmed ! in the type prefix if API or client breaking change
  • targeted the correct branch
  • provided a link to the relevant issue or specification
  • reviewed "Files changed" and left comments if necessary
  • included the necessary unit and integration tests
  • updated the relevant documentation or specification, including comments for documenting Go code
  • confirmed all CI checks have passed

Reviewers 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.

I have...

  • confirmed the correct type prefix in the PR title
  • confirmed all author checklist items have been addressed
  • reviewed state machine logic, API design and naming, documentation is accurate, tests and test coverage

@beer-1 beer-1 self-assigned this Sep 16, 2025
@beer-1 beer-1 requested a review from a team as a code owner September 16, 2025 03:20
Copy link

coderabbitai bot commented Sep 16, 2025

📝 Walkthrough

Walkthrough

This change removes the delegation migration feature across mstaking and Move DEX: deletes proto RPCs/messages, generated code, keeper methods/fields, CLI/query handlers, Move migration keeper and contract, related events/types, and tests. Wiring updates drop BalancerMigrationKeeper and adjust NewKeeper signatures in app setup and tests.

Changes

Cohort / File(s) Summary
Proto API removals
proto/initia/mstaking/v1/query.proto, proto/initia/mstaking/v1/staking.proto, proto/initia/mstaking/v1/tx.proto, api/initia/mstaking/v1/query.pulsar.go, api/initia/mstaking/v1/staking.pulsar.go
Deleted Migration RPC and messages; removed DelegationMigration message; regenerated code drops related types, descriptors, and reflection; message counts reduced.
mstaking keeper: migration removal
x/mstaking/keeper/keeper.go, x/mstaking/keeper/migration.go, x/mstaking/keeper/msg_server.go, x/mstaking/keeper/grpc_query.go, x/mstaking/keeper/grpc_query_test.go, x/mstaking/keeper/migration_test.go, x/mstaking/keeper/common_test.go
Removed DexMigrationKeeper field, Migrations store, RegisterMigration/MigrateDelegation handlers and logic, gRPC Migration query; deleted migration implementation and tests; updated NewKeeper signature.
mstaking types and CLI cleanup
x/mstaking/types/events.go, x/mstaking/types/tx.go, x/mstaking/types/tx_test.go, x/mstaking/types/codec.go, x/mstaking/types/delegation.go, x/mstaking/types/expected_keepers.go, x/mstaking/client/cli/query.go
Deleted migration events/attributes, msg types validations and test cases, legacy registrations, DelegationMigration.String, DexMigrationKeeper interface; removed CLI migration query command.
Move DEX migration removal
x/move/keeper/balancer_migration.go, x/move/keeper/balancer_migration_test.go, x/move/keeper/contracts/sources/DexMigration.move, x/move/keeper/common_test.go, x/move/keeper/balancer.go, x/move/types/connector.go
Deleted BalancerMigrationKeeper and public migration methods, associated tests, and test Move module; removed fee-rate accessors and Dex migration constant/parser function.
App wiring and tests update
app/keepers/keepers.go, x/bank/keeper/common_test.go, x/distribution/keeper/common_test.go, x/dynamic-fee/keeper/common_test.go, x/gov/keeper/common_test.go, x/ibc-hooks/keeper/common_test.go, x/reward/keeper/common_test.go, x/slashing/keeper/common_test.go
Dropped BalancerMigrationKeeper argument from staking keeper construction; updated calls to NewKeeper to match new signature; retained VotingPowerKeeper.
Docs
spec/mstaking/migrate_delegation/readme.md
Removed migration feature specification document.

Sequence Diagram(s)

sequenceDiagram
  autonumber
  participant Client
  participant MsgServer
  participant StakingKeeper
  participant DexMigrationKeeper
  participant MoveVM

  rect rgb(245, 248, 255)
  note over Client,StakingKeeper: Before (legacy flow)
  Client->>MsgServer: MsgMigrateDelegation
  MsgServer->>StakingKeeper: MigrateDelegation(...)
  StakingKeeper->>DexMigrationKeeper: MigrateLP(...)
  DexMigrationKeeper->>MoveVM: Execute migration calls
  MoveVM-->>DexMigrationKeeper: LP out
  DexMigrationKeeper-->>StakingKeeper: result
  StakingKeeper-->>MsgServer: shares updated
  MsgServer-->>Client: MsgMigrateDelegationResponse
  end
Loading
sequenceDiagram
  autonumber
  participant Client
  participant QueryServer
  participant StakingKeeper

  rect rgb(255, 245, 245)
  note over Client,StakingKeeper: After (current)
  Client -x QueryServer: Migration(...) [removed]
  QueryServer-->>Client: Not available
  end
Loading

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~25 minutes

Poem

I nibble the code where migrations once grew,
Paths pruned clean, fewer trails to go through.
Hops in the grass, my whiskers unfrayed—
Less maze, more meadow where queries once stayed.
Thump-thump—onward! The garden’s renewed. 🐇✨

Pre-merge checks and finishing touches

❌ Failed checks (1 warning)
Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 16.67% which is insufficient. The required threshold is 80.00%. You can run @coderabbitai generate docstrings to improve docstring coverage.
✅ Passed checks (2 passed)
Check name Status Explanation
Title Check ✅ Passed The title succinctly and accurately summarizes the primary change: removal of the delegation migration feature. It is concise, uses the conventional "feat:" prefix, and directly matches the changeset which deletes migration-related protos, keeper methods, Move contracts, CLI commands, and tests. This makes it clear to a teammate scanning history what the PR does.
Description Check ✅ Passed The PR description states the intent to remove the delegation migration flow, which is directly related to the extensive deletions in the diff. The description is accurate but minimal and does not enumerate the scope of removed public APIs, tests, and docs. Because multiple public proto messages, RPCs, keeper APIs, Move modules, and CLI commands were removed, the description should explicitly note the API-breaking impact and any follow-up or migration guidance.
✨ Finishing touches
  • 📝 Generate Docstrings
🧪 Generate unit tests
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch feat/remove-delegation-migration

Tip

👮 Agentic pre-merge checks are now available in preview!

Pro plan users can now enable pre-merge checks in their settings to enforce checklists before merging PRs.

  • Built-in checks – Quickly apply ready-made checks to enforce title conventions, require pull request descriptions that follow templates, validate linked issues for compliance, and more.
  • Custom agentic checks – Define your own rules using CodeRabbit’s advanced agentic capabilities to enforce organization-specific policies and workflows. For example, you can instruct CodeRabbit’s agent to verify that API documentation is updated whenever API schema files are modified in a PR. Note: Upto 5 custom checks are currently allowed during the preview period. Pricing for this feature will be announced in a few weeks.

Please see the documentation for more information.

Example:

reviews:
  pre_merge_checks:
    custom_checks:
      - name: "Undocumented Breaking Changes"
        mode: "warning"
        instructions: |
          Pass/fail criteria: All breaking changes to public APIs, CLI flags, environment variables, configuration keys, database schemas, or HTTP/GraphQL endpoints must be documented in the "Breaking Change" section of the PR description and in CHANGELOG.md. Exclude purely internal or private changes (e.g., code not exported from package entry points or explicitly marked as internal).

Please share your feedback with us on this Discord post.


Comment @coderabbitai help to get the list of available commands and usage tips.

Copy link

⚠️ Deprecation Warning: The deny-licenses option is deprecated for possible removal in the next major release. For more information, see actions/dependency-review-action/issues/938.

Dependency Review

✅ No vulnerabilities or license issues or OpenSSF Scorecard issues found.

Scanned Files

None

Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 0

🧹 Nitpick comments (1)
x/mstaking/keeper/keeper.go (1)

32-36: Keeper fields cleanup — dexMigration/Migrations removed; unexporting VotingPowerKeeper is optional

  • No references to dexMigrationKeeper or Migrations found — safe to remove.
  • VotingPowerKeeper is still used across the repo (x/mstaking/keeper/keeper.go:34, 78, 104; x/mstaking/keeper/voting_power.go:16; x/mstaking/keeper/val_state_change_test.go:62; app/keepers/keepers.go:250). Unexporting the field is a low-priority refactor — only do it if you also update any direct assignments/usages (tests or other packages) that access StakingKeeper.VotingPowerKeeper.
📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

Cache: Disabled due to data retention organization setting

Knowledge base: Disabled due to data retention organization setting

📥 Commits

Reviewing files that changed from the base of the PR and between b7ba56f and 0571051.

⛔ Files ignored due to path filters (7)
  • api/initia/mstaking/v1/query_grpc.pb.go is excluded by !**/*.pb.go, !**/*.pb.go
  • api/initia/mstaking/v1/tx_grpc.pb.go is excluded by !**/*.pb.go, !**/*.pb.go
  • client/docs/swagger-ui/swagger-initia.yaml is excluded by !**/*.yaml
  • x/mstaking/types/query.pb.go is excluded by !**/*.pb.go, !**/*.pb.go
  • x/mstaking/types/query.pb.gw.go is excluded by !**/*.pb.gw.go, !**/*.pb.gw.go
  • x/mstaking/types/staking.pb.go is excluded by !**/*.pb.go, !**/*.pb.go
  • x/mstaking/types/tx.pb.go is excluded by !**/*.pb.go, !**/*.pb.go
📒 Files selected for processing (34)
  • api/initia/mstaking/v1/query.pulsar.go (4 hunks)
  • api/initia/mstaking/v1/staking.pulsar.go (4 hunks)
  • app/keepers/keepers.go (0 hunks)
  • proto/initia/mstaking/v1/query.proto (0 hunks)
  • proto/initia/mstaking/v1/staking.proto (0 hunks)
  • proto/initia/mstaking/v1/tx.proto (0 hunks)
  • spec/mstaking/migrate_delegation/readme.md (0 hunks)
  • x/bank/keeper/common_test.go (0 hunks)
  • x/distribution/keeper/common_test.go (0 hunks)
  • x/dynamic-fee/keeper/common_test.go (0 hunks)
  • x/gov/keeper/common_test.go (0 hunks)
  • x/ibc-hooks/keeper/common_test.go (0 hunks)
  • x/move/keeper/balancer.go (0 hunks)
  • x/move/keeper/balancer_migration.go (0 hunks)
  • x/move/keeper/balancer_migration_test.go (0 hunks)
  • x/move/keeper/common_test.go (0 hunks)
  • x/move/keeper/contracts/sources/DexMigration.move (0 hunks)
  • x/move/types/connector.go (0 hunks)
  • x/mstaking/client/cli/query.go (0 hunks)
  • x/mstaking/keeper/common_test.go (0 hunks)
  • x/mstaking/keeper/grpc_query.go (0 hunks)
  • x/mstaking/keeper/grpc_query_test.go (0 hunks)
  • x/mstaking/keeper/keeper.go (2 hunks)
  • x/mstaking/keeper/migration.go (0 hunks)
  • x/mstaking/keeper/migration_test.go (0 hunks)
  • x/mstaking/keeper/msg_server.go (0 hunks)
  • x/mstaking/types/codec.go (0 hunks)
  • x/mstaking/types/delegation.go (0 hunks)
  • x/mstaking/types/events.go (1 hunks)
  • x/mstaking/types/expected_keepers.go (0 hunks)
  • x/mstaking/types/tx.go (0 hunks)
  • x/mstaking/types/tx_test.go (0 hunks)
  • x/reward/keeper/common_test.go (0 hunks)
  • x/slashing/keeper/common_test.go (0 hunks)
💤 Files with no reviewable changes (30)
  • x/mstaking/types/codec.go
  • x/mstaking/types/delegation.go
  • x/ibc-hooks/keeper/common_test.go
  • x/move/types/connector.go
  • x/mstaking/client/cli/query.go
  • x/mstaking/keeper/migration_test.go
  • x/move/keeper/balancer.go
  • x/move/keeper/common_test.go
  • spec/mstaking/migrate_delegation/readme.md
  • proto/initia/mstaking/v1/staking.proto
  • x/move/keeper/balancer_migration_test.go
  • x/slashing/keeper/common_test.go
  • proto/initia/mstaking/v1/query.proto
  • proto/initia/mstaking/v1/tx.proto
  • x/mstaking/keeper/grpc_query.go
  • x/mstaking/keeper/grpc_query_test.go
  • app/keepers/keepers.go
  • x/mstaking/types/expected_keepers.go
  • x/distribution/keeper/common_test.go
  • x/mstaking/types/tx_test.go
  • x/mstaking/keeper/msg_server.go
  • x/reward/keeper/common_test.go
  • x/mstaking/keeper/migration.go
  • x/mstaking/keeper/common_test.go
  • x/bank/keeper/common_test.go
  • x/move/keeper/contracts/sources/DexMigration.move
  • x/mstaking/types/tx.go
  • x/dynamic-fee/keeper/common_test.go
  • x/move/keeper/balancer_migration.go
  • x/gov/keeper/common_test.go
⏰ 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). (3)
  • GitHub Check: Run test and upload codecov
  • GitHub Check: golangci-lint
  • GitHub Check: Analyze (go)
🔇 Additional comments (11)
x/mstaking/types/events.go (1)

14-22: No internal references to removed migration events/attributes — resolved

Repository-wide search (including x/mstaking/types/events.go) found no matches for the removed migration event/attribute identifiers. If external consumers (indexers/docs/other repos) depend on them, add a BREAKING CHANGE note to the PR/release notes.

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

100-107: mstaking: add nil-guards for required keepers and verify callers

Add explicit panics if AccountKeeper/BankKeeper/VotingPowerKeeper are nil and update callers; re-run a repo-wide search to ensure all NewKeeper call sites are correct.

Apply:

 func NewKeeper(
   cdc codec.Codec,
   storeService corestoretypes.KVStoreService,
   ak types.AccountKeeper,
   bk types.BankKeeper,
   vk types.VotingPowerKeeper,
   authority string,
   validatorAddressCodec addresscodec.Codec,
   consensusAddressCodec addresscodec.Codec,
 ) *Keeper {
+  if ak == nil || bk == nil || vk == nil {
+    panic("mstaking: AccountKeeper/BankKeeper/VotingPowerKeeper must not be nil")
+  }

Check callers (examples): app/keepers/keepers.go:245, x/mstaking/keeper/common_test.go:351 (also many x//keeper/_test.go files). Re-run:
rg -nP -C2 '\bstakingkeeper.NewKeeper\s*('

api/initia/mstaking/v1/query.pulsar.go (5)

16046-16046: TypeBuilder NumMessages=28 — LGTM.

Matches the msgTypes length; consistent with the feature removal.


15590-15630: LGTM — MsgTypes/goTypes slice sizes verified (28)
make([]protoimpl.MessageInfo, 28) and NumMessages: 28 confirmed; indices reflowed consistently.


13887-13901: Do not hand-edit generated files — regenerate from the proto and commit the output.

api/initia/mstaking/v1/query.pulsar.go is generated by protoc-gen-go-pulsar; regenerate it from proto/initia/mstaking/v1/query.proto using the repo's proto generation pipeline (Makefile target that invokes scripts/protocgen-pulsar.sh or run sh ./scripts/protocgen-pulsar.sh) and commit the regenerated file to avoid drift.

Verified: query.pulsar.go is present and tracked; proto/initia/mstaking/v1/query.proto and scripts/protocgen-pulsar.sh exist. Generation could not be executed here (make absent and buf gen template not found) — run the repo generation locally/CI and confirm there are no manual edits left.


15631-15694: depIdxs remapped — verified no stale type references. depIdxs block in api/initia/mstaking/v1/query.pulsar.go (lines 15620–15694) maps to valid types and contains no references to "QueryMigration" or "Migration".


15382-15575: Migration RPCs/HTTP routes fully purged — verified. No RPCs or google.api.http annotations matching "migration", "migrate", or "delegation_migration" were found in proto/ or generated Go query files. Only remaining mention is a comment in x/move/keeper/keeper.go (around lines 203–206) — not an RPC/HTTP route.

api/initia/mstaking/v1/staking.pulsar.go (4)

12664-12674: Pin and surface protobuf toolchain versions for reproducible builds.

api/initia/mstaking/v1/staking.pulsar.go header shows protoc-gen-go v1.27.0 and protoc (unknown). Pin protoc, protoc-gen-go, and protoc-gen-go-pulsar to avoid non-deterministic generated-code deltas. Recommended approaches:

  • buf managed mode: add buf.gen.yaml + buf.lock
  • or tools.go + Makefile targets to pin generator versions

Quick check found no buf.gen.yaml, buf.lock, or tools.go in the repo — verify the proto generation workflow and add pinning artifacts.


14445-14449: Type-index shifts — regenerate all pulsar-generated files in api/initia/mstaking/v1

Indices for google.protobuf.Any and cosmos.base.v1beta1.{Coin,DecCoin} appear shifted in api/initia/mstaking/v1/staking.pulsar.go (lines 14445–14449). Ensure every *_pulsar.go in this package was regenerated together; if not, re-run the protobuf + pulsar codegen and commit the updated files. Verify by comparing the goTypes/type-index slice ordering and the generator headers across all generated files.


14421-14421: Breaking change risk — proto message count reduced to 19: verify & document

Removing public proto messages shifts reflection indices and breaks downstream clients.

  • Document the exact removals as a breaking change in CHANGELOG/UPGRADE (list removed message names and date) and gate behind a release plan.
  • Run a buf breaking check against the last release tag and attach the output. Example:
    buf breaking --against '.git#ref=<last_release_tag>'
  • Re-run repo searches locally (CI/sandbox produced no output) and paste results to confirm no lingering references or generated-index shifts. Run:
    rg -nP 'DelegationMigration\b|MsgRegisterMigration\b|MsgMigrateDelegation\b|RegisterMigration\b|MigrateDelegation\b|MigrationKeeper\b|BalancerMigrationKeeper\b' || true
    
    rg -nP 'file_initia_mstaking_v1_staking_proto_msgTypes\s*=\s*make\(\[]protoimpl.MessageInfo,\s*\d+\)' api/initia/mstaking/v1/staking.pulsar.go -n -A2 || true
    
    rg -nP 'md_\w+\s*=\s*File_initia_mstaking_v1_staking_proto\.Messages' api/initia/mstaking/v1/staking.pulsar.go -n -A2 || true
    

Location: api/initia/mstaking/v1/staking.pulsar.go:14421 (also applies to 14730-14731)

var file_initia_mstaking_v1_staking_proto_msgTypes = make([]protoimpl.MessageInfo, 19)

14373-14405: Generated descriptor bytes changed — DelegationMigration removal verified; confirm regen toolchain

  • No occurrences of "DelegationMigration" found in proto/ or elsewhere in the repo (rg search).
  • api/initia/mstaking/v1/staking.pulsar.go has the standard "Code generated by protoc-gen-go-pulsar. DO NOT EDIT." header and only raw descriptor bytes present.
  • Cannot determine the exact protoc/protoc-gen-go-pulsar version from the file — confirm the file was regenerated with the pinned toolchain and contains no hand edits.

Copy link

codecov bot commented Sep 16, 2025

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 41.59%. Comparing base (b7ba56f) to head (0571051).

Additional details and impacted files

Impacted file tree graph

@@            Coverage Diff             @@
##             main     #438      +/-   ##
==========================================
- Coverage   41.66%   41.59%   -0.07%     
==========================================
  Files         318      316       -2     
  Lines       29743    29295     -448     
==========================================
- Hits        12391    12184     -207     
+ Misses      15490    15290     -200     
+ Partials     1862     1821      -41     
Files with missing lines Coverage Δ
app/keepers/keepers.go 98.44% <ø> (-0.01%) ⬇️
x/move/keeper/balancer.go 46.91% <ø> (-0.91%) ⬇️
x/move/types/connector.go 21.59% <ø> (+1.31%) ⬆️
x/mstaking/client/cli/query.go 0.00% <ø> (ø)
x/mstaking/keeper/grpc_query.go 48.89% <ø> (-0.02%) ⬇️
x/mstaking/keeper/keeper.go 75.55% <100.00%> (-0.79%) ⬇️
x/mstaking/keeper/msg_server.go 46.21% <ø> (+5.63%) ⬆️
x/mstaking/types/codec.go 53.33% <ø> (+0.39%) ⬆️
x/mstaking/types/delegation.go 51.18% <ø> (+1.41%) ⬆️
x/mstaking/types/tx.go 55.55% <ø> (-4.54%) ⬇️

... and 1 file with indirect coverage changes

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant