Conversation
📝 WalkthroughWalkthroughReplaces v9.5 upgrade wiring with v9.6: updates protocol/app/upgrades.go imports and registrations, removes the v9.5 container test, adds v9.6 constants, upgrade handler, and a v9.6 container test scaffold, and bumps testnet version files from v9.5→v9.6 and pre-upgrade references v9.4→v9.5. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
participant Testnet as Testnet (containertest)
participant UpgradeRegistrar as App upgrades.go
participant UpgHandler as v9.6 CreateUpgradeHandler
participant ModuleMgr as module.Manager
participant Store as KV Stores
Note over Testnet,UpgradeRegistrar: pre-upgrade phase
Testnet->>UpgradeRegistrar: start node with current binary
UpgradeRegistrar->>UpgHandler: register handler for "v9.6"
Testnet->>UpgHandler: trigger upgrade height / plan
UpgHandler->>ModuleMgr: mm.RunMigrations(ctx, configurator, vm)
ModuleMgr->>Store: migrate stores / run module migrations
Store-->>ModuleMgr: migration results / version map
ModuleMgr-->>UpgHandler: return new version map / error?
UpgHandler-->>Testnet: upgrade success / failure
Note right of Testnet: post-upgrade validation hooks run
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 1 | ❌ 2❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (1 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
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. Comment |
|
Can you test this in staging? |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Fix all issues with AI Agents
In @protocol/app/upgrades.go:
- Around line 16-18: The Upgrades slice currently only registers v_9_6.Upgrade
(Upgrades = []upgrades.Upgrade{ v_9_6.Upgrade }) which may break genesis/replay
paths if historical upgrade handlers (v9.5, v9.4, etc.) are required; decide
whether the project intends to register only the latest upgrade or to support
full genesis/replay, then either (A) append the necessary historic upgrade
values (e.g., v_9_4.Upgrade, v_9_5.Upgrade, ...) into the Upgrades slice so all
handlers are registered in order, or (B) explicitly document the strategy in
comments and ensure an alternate mechanism applies historical migrations during
genesis/replay; update the Upgrades slice and/or comments accordingly and
reference the v_9_6.Upgrade symbol as the current entry when making the change.
In @protocol/app/upgrades/v9.6/constants.go:
- Around line 1-15: The PR description is incomplete; update the changelist and
test plan for the v9.6 upgrade by adding a clear "Changelist" that enumerates
the changes introduced in this upgrade (features, bug fixes, breaking changes)
and a "Test Plan" that describes how v9.6 was validated
(unit/integration/upgrade tests, migration verification steps, and any manual
checks), and note any coordination required for breaking changes; reference the
UpgradeName constant and the Upgrade variable in constants.go
(v_9_6.UpgradeName, v_9_6.Upgrade) when describing the release tag and ensure
the PR body documents expected runtime effects and rollback/rollback-test
instructions if applicable.
🧹 Nitpick comments (1)
protocol/app/upgrades/v9.6/upgrade_container_test.go (1)
30-34: Consider adding validation logic or documenting intent.The empty helper functions mean this test only verifies the upgrade completes without crashing, but doesn't validate any state transitions or behavior changes.
If v9.6 is a simple version bump with no state migrations or breaking changes, these empty helpers are fine. However, if there are specific behaviors to validate (parameter changes, state migrations, new features), consider adding checks to
postUpgradeChecks.Alternatively, if these are intentionally minimal, you could remove the unused helpers to simplify the test.
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (8)
protocol/app/upgrades.goprotocol/app/upgrades/v9.5/upgrade_container_test.goprotocol/app/upgrades/v9.6/constants.goprotocol/app/upgrades/v9.6/upgrade.goprotocol/app/upgrades/v9.6/upgrade_container_test.goprotocol/testing/version/VERSION_CURRENTprotocol/testing/version/VERSION_FULL_NAME_PREUPGRADEprotocol/testing/version/VERSION_PREUPGRADE
💤 Files with no reviewable changes (1)
- protocol/app/upgrades/v9.5/upgrade_container_test.go
🧰 Additional context used
🧠 Learnings (3)
📓 Common learnings
Learnt from: hwray
Repo: dydxprotocol/v4-chain PR: 2597
File: indexer/services/ender/src/scripts/handlers/dydx_update_perpetual_v1_handler.sql:16-20
Timestamp: 2024-11-22T18:12:04.606Z
Learning: Avoid suggesting changes to deprecated functions such as `dydx_update_perpetual_v1_handler` in `indexer/services/ender/src/scripts/handlers/dydx_update_perpetual_v1_handler.sql` if they are unchanged in the PR.
📚 Learning: 2025-07-09T20:03:51.367Z
Learnt from: anmolagrawal345
Repo: dydxprotocol/v4-chain PR: 2966
File: protocol/testing/containertest/preupgrade_genesis.json:997-999
Timestamp: 2025-07-09T20:03:51.367Z
Learning: The preupgrade genesis file `protocol/testing/containertest/preupgrade_genesis.json` is fetched from a localnet and should be used directly without parameterization. Hard-coded IP addresses in this file are expected since they represent the actual network configuration from the localnet snapshot used for upgrade testing.
Applied to files:
protocol/app/upgrades/v9.6/upgrade_container_test.go
📚 Learning: 2025-05-24T00:45:00.519Z
Learnt from: anmolagrawal345
Repo: dydxprotocol/v4-chain PR: 2837
File: proto/dydxprotocol/clob/order.proto:4-4
Timestamp: 2025-05-24T00:45:00.519Z
Learning: In the dydxprotocol/v4-chain repository, cosmos_proto dependencies are managed through buf.yaml configuration rather than being physically present in the repository. The import "cosmos_proto/cosmos.proto" is a standard pattern used across 28+ proto files for cosmos address annotations like (cosmos_proto.scalar) = "cosmos.AddressString".
Applied to files:
protocol/app/upgrades.go
🧬 Code graph analysis (2)
protocol/app/upgrades.go (3)
protocol/app/upgrades/v9.6/constants.go (2)
Upgrade(12-15)UpgradeName(9-9)protocol/app/upgrades/v9.5/constants.go (2)
Upgrade(12-15)UpgradeName(9-9)protocol/app/upgrades/v9.6/upgrade.go (1)
CreateUpgradeHandler(12-22)
protocol/app/upgrades/v9.6/upgrade.go (2)
protocol/lib/context.go (1)
UnwrapSDKContext(31-55)protocol/app/upgrades/v9.6/constants.go (1)
UpgradeName(9-9)
⏰ 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). (28)
- GitHub Check: (Mainnet) Build and Push ECS Services / call-build-and-push-auxo-lambda / (auxo) Build and Push Lambda
- GitHub Check: (Mainnet) Build and Push ECS Services / call-build-and-push-ecs-service-socks / (socks) Build and Push
- GitHub Check: (Mainnet) Build and Push ECS Services / call-build-and-push-ecs-service-ender / (ender) Build and Push
- GitHub Check: (Mainnet) Build and Push ECS Services / call-build-and-push-bazooka-lambda / (bazooka) Build and Push Lambda
- GitHub Check: (Public Testnet) Build and Push ECS Services / call-build-and-push-ecs-service-roundtable / (roundtable) Build and Push
- GitHub Check: (Public Testnet) Build and Push ECS Services / call-build-and-push-ecs-service-comlink / (comlink) Build and Push
- GitHub Check: (Public Testnet) Build and Push ECS Services / call-build-and-push-auxo-lambda / (auxo) Build and Push Lambda
- GitHub Check: (Mainnet) Build and Push ECS Services / call-build-and-push-ecs-service-comlink / (comlink) Build and Push
- GitHub Check: (Mainnet) Build and Push ECS Services / call-build-and-push-ecs-service-roundtable / (roundtable) Build and Push
- GitHub Check: (Mainnet) Build and Push ECS Services / call-build-and-push-vulcan / (vulcan) Build and Push
- GitHub Check: (Public Testnet) Build and Push ECS Services / call-build-and-push-vulcan / (vulcan) Build and Push
- GitHub Check: (Public Testnet) Build and Push ECS Services / call-build-and-push-ecs-service-socks / (socks) Build and Push
- GitHub Check: (Public Testnet) Build and Push ECS Services / call-build-and-push-ecs-service-ender / (ender) Build and Push
- GitHub Check: (Public Testnet) Build and Push ECS Services / call-build-and-push-bazooka-lambda / (bazooka) Build and Push Lambda
- GitHub Check: test-coverage-upload
- GitHub Check: liveness-test
- GitHub Check: unit-end-to-end-and-integration
- GitHub Check: test-race
- GitHub Check: build
- GitHub Check: benchmark
- GitHub Check: golangci-lint
- GitHub Check: check-sample-pregenesis-up-to-date
- GitHub Check: build-and-push-mainnet
- GitHub Check: container-tests
- GitHub Check: build-and-push-testnet
- GitHub Check: Analyze (go)
- GitHub Check: Analyze (javascript-typescript)
- GitHub Check: Summary
🔇 Additional comments (11)
protocol/testing/version/VERSION_PREUPGRADE (1)
1-1: LGTM! Pre-upgrade version correctly updated.The pre-upgrade version has been properly bumped to v9.5, which aligns with the v9.6 upgrade target.
protocol/testing/version/VERSION_CURRENT (1)
1-1: LGTM! Current version correctly bumped to v9.6.The target upgrade version has been properly set to v9.6.
protocol/testing/version/VERSION_FULL_NAME_PREUPGRADE (1)
1-1: LGTM! Full pre-upgrade version correctly updated.The full semantic version has been properly updated to v9.5.0, consistent with the other version files.
protocol/app/upgrades.go (2)
6-6: LGTM! Import correctly updated to v9.6.The import has been properly updated to reference the v9.6 upgrade package.
24-35: The upgrade handler wiring is correct and the minimal implementation is intentional.The v9.6 upgrade handler is appropriately minimal—it performs only standard module migrations via
mm.RunMigrations()with no custom state transformations or store upgrades. This is consistent with a simple version bump where no state-breaking changes require migration logic. Unlike v9.5 (which included custom migration for affiliate stats), v9.6 has no similar requirements, as confirmed by the absence of TODOs or incomplete work indicators in the upgrade files.protocol/app/upgrades/v9.6/constants.go (3)
1-6: LGTM! Package and imports are correctly defined.The package name and imports are properly set up for the v9.6 upgrade definition.
8-10: LGTM! UpgradeName correctly set to v9.6.The upgrade name constant is properly defined and matches the target version.
12-15: LGTM! Upgrade definition is consistent.The
Upgradevariable is properly initialized with the upgrade name and store upgrades. The emptyStoreUpgrades{}is consistent with the verification request made in the upgrades.go review regarding the minimal upgrade logic.protocol/app/upgrades/v9.6/upgrade.go (2)
1-10: LGTM! Standard upgrade handler setup.The package declaration and imports are appropriate for a standard Cosmos SDK upgrade handler.
12-22: Standard minimal upgrade handler - verify no migrations needed.The implementation correctly follows the Cosmos SDK upgrade pattern. This is a straightforward handler that only runs standard module migrations without any custom logic.
If this upgrade requires specific state migrations, store changes, or parameter updates, you'll want to add that logic before or after the
RunMigrationscall. Otherwise, this minimal approach is appropriate for a simple version bump.What are the typical migration requirements for dYdX v4-chain protocol upgrades?protocol/app/upgrades/v9.6/upgrade_container_test.go (1)
12-28: Test structure looks good.The test correctly follows the established container test pattern for upgrades. It verifies that the v9.6 upgrade can complete without errors on a testnet.
Note that this is currently a minimal smoke test - it confirms the upgrade runs successfully but doesn't validate specific state changes or migrations.
| Upgrades = []upgrades.Upgrade{ | ||
| v_9_5.Upgrade, | ||
| v_9_6.Upgrade, | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Description: Check if historical upgrades are typically maintained or removed in upgrade cycles
# Check if other upgrade version directories exist
echo "=== Checking for other upgrade version directories ==="
fd -t d 'v[0-9]+\.[0-9]+' protocol/app/upgrades/
# Check git history to see if multiple upgrades were ever present in Upgrades slice
echo -e "\n=== Checking git history of upgrades.go for Upgrades slice pattern ==="
git log --oneline --all -- protocol/app/upgrades.go | head -20
# Search for any references to older upgrade versions in the codebase
echo -e "\n=== Searching for v9.5 references that might need cleanup ==="
rg -n "v_9_5|v9\.5" --type go -g '!vendor/' -g '!*.sum' || echo "No v9.5 references found"
# Check actual imports and usage in the upgrades.go file
echo -e "\n=== Checking current upgrades.go for imports and Upgrades slice ==="
cat protocol/app/upgrades.go | head -50Repository: dydxprotocol/v4-chain
Length of output: 3661
Confirm the upgrade registration strategy for genesis and replay scenarios.
The Upgrades slice contains only v_9_6.Upgrade, while historical upgrade packages (v9.5, v9.4, etc.) remain defined in protocol/app/upgrades/ but are not registered in the active slice. This matches the codebase's pattern where each release registers only the latest upgrade.
For nodes syncing from genesis or replaying historical state transitions, ensure this is the intended behavior. If full genesis syncs are supported, historical upgrade handlers may need to remain registered.
🤖 Prompt for AI Agents
In @protocol/app/upgrades.go around lines 16 - 18, The Upgrades slice currently
only registers v_9_6.Upgrade (Upgrades = []upgrades.Upgrade{ v_9_6.Upgrade })
which may break genesis/replay paths if historical upgrade handlers (v9.5, v9.4,
etc.) are required; decide whether the project intends to register only the
latest upgrade or to support full genesis/replay, then either (A) append the
necessary historic upgrade values (e.g., v_9_4.Upgrade, v_9_5.Upgrade, ...) into
the Upgrades slice so all handlers are registered in order, or (B) explicitly
document the strategy in comments and ensure an alternate mechanism applies
historical migrations during genesis/replay; update the Upgrades slice and/or
comments accordingly and reference the v_9_6.Upgrade symbol as the current entry
when making the change.
| package v_9_6 | ||
|
|
||
| import ( | ||
| store "cosmossdk.io/store/types" | ||
| "github.com/dydxprotocol/v4-chain/protocol/app/upgrades" | ||
| ) | ||
|
|
||
| const ( | ||
| UpgradeName = "v9.6" | ||
| ) | ||
|
|
||
| var Upgrade = upgrades.Upgrade{ | ||
| UpgradeName: UpgradeName, | ||
| StoreUpgrades: store.StoreUpgrades{}, | ||
| } |
There was a problem hiding this comment.
Complete the PR description with changelist and test plan.
The PR description has placeholder text for required sections:
- Changelist: "[Describe or list the changes made in this PR]"
- Test Plan: "[Describe how this PR was tested (if applicable)]"
For an upgrade PR, it's essential to document:
- What changes are included in v9.6 (new features, bug fixes, breaking changes)
- How the upgrade was tested (upgrade tests, migration verification, etc.)
- Whether this is a breaking upgrade requiring special coordination
This documentation helps reviewers understand the scope and helps future maintainers understand the upgrade history.
🤖 Prompt for AI Agents
In @protocol/app/upgrades/v9.6/constants.go around lines 1 - 15, The PR
description is incomplete; update the changelist and test plan for the v9.6
upgrade by adding a clear "Changelist" that enumerates the changes introduced in
this upgrade (features, bug fixes, breaking changes) and a "Test Plan" that
describes how v9.6 was validated (unit/integration/upgrade tests, migration
verification steps, and any manual checks), and note any coordination required
for breaking changes; reference the UpgradeName constant and the Upgrade
variable in constants.go (v_9_6.UpgradeName, v_9_6.Upgrade) when describing the
release tag and ensure the PR body documents expected runtime effects and
rollback/rollback-test instructions if applicable.
|
@mergify backport release/protocol/v9.x |
✅ Backports have been createdDetails
|
(cherry picked from commit f4db85a)
2 participants
Changelist
Upgrade 9.6 to add changing tick size + step size by means of gov proposal.
Test Plan
Will test in testnet.
Author/Reviewer Checklist
state-breakinglabel.indexer-postgres-breakinglabel.PrepareProposalorProcessProposal, manually add the labelproposal-breaking.feature:[feature-name].backport/[branch-name].refactor,chore,bug.Summary by CodeRabbit
Chores
New Features
Bug Fixes / Behavior
Tests
✏️ Tip: You can customize this high-level summary in your review settings.