Skip to content

Conversation

@jusbar23
Copy link
Contributor

@jusbar23 jusbar23 commented Oct 20, 2025

Changelist

  • Add migrations for Affiliate whitelist map
  • Update the affiliate tiers to sliding affiliate scheme
  • Set default affiliate params

Test Plan

  • Unit tests

Author/Reviewer Checklist

  • If this PR has changes that result in a different app state given the same prior state and transaction list, manually add the state-breaking label.
  • If the PR has breaking postgres changes to the indexer add the indexer-postgres-breaking label.
  • If this PR isn't state-breaking but has changes that modify behavior in PrepareProposal or ProcessProposal, manually add the label proposal-breaking.
  • If this PR is one of many that implement a specific feature, manually label them all feature:[feature-name].
  • If you wish to for mergify-bot to automatically create a PR to backport your change to a release branch, manually add the label backport/[branch-name].
  • Manually add any of the following labels: refactor, chore, bug.

Summary by CodeRabbit

  • Chores

    • Protocol upgraded to v9.4; upgrade wiring updated to schedule and run the v9.4 migration and register its handler.
    • Affiliate initialization refined: new default tiers, parameters, and whitelist metadata added and applied during the upgrade; affiliate overrides are migrated.
    • Version identifiers in test artifacts and a sample genesis file updated for v9.4.
  • Tests

    • New state-upgrade tests validating v9.4 upgrade flow and affiliate state transitions (pre-upgrade → defaults/overrides).

@coderabbitai
Copy link
Contributor

coderabbitai bot commented Oct 20, 2025

Walkthrough

Replaces v9.3 upgrade wiring with v9.4: adds v9.4 constants, upgrade handler and tests, updates app upgrade registration to use v9.4 and passes app.AffiliatesKeeper into CreateUpgradeHandler, and bumps test/version strings and sample genesis version.

Changes

Cohort / File(s) Change Summary
Upgrade wiring
protocol/app/upgrades.go
Swap registration from v9.3v9.4, check duplicate handler for v9.4, and call v9.4.CreateUpgradeHandler with app.AffiliatesKeeper.
v9.4 constants
protocol/app/upgrades/v9.4/constants.go
New file: defines Upgrade and UpgradeName ("v9.4"), plus PreviousAffiliate* and DefaultAffiliate* tiers/parameters/whitelist used during migration.
v9.4 upgrade handler
protocol/app/upgrades/v9.4/upgrade.go
New CreateUpgradeHandler(mm, affiliateKeeper, configurator) returning an upgradetypes.UpgradeHandler; adds helper funcs to set default tiers/parameters and migrate overrides; runs mm.RunMigrations.
v9.4 tests
protocol/app/upgrades/v9.4/upgrade_container_test.go
New container test: verifies pre-upgrade affiliate state, runs the upgrade, asserts post-upgrade default tiers/parameters and migrated overrides.
Test/version strings
protocol/testing/version/VERSION_CURRENT, protocol/testing/version/VERSION_PREUPGRADE, protocol/testing/version/VERSION_FULL_NAME_PREUPGRADE
Bump test/version constants: VERSION_CURRENTv9.4, VERSION_PREUPGRADEv9.3, VERSION_FULL_NAME_PREUPGRADEv9.3.
Genesis sample
protocol/scripts/genesis/sample_pregenesis.json
Update app_version string value.

Sequence Diagram(s)

sequenceDiagram
    participant App
    participant UpgradesSetup as "SetupUpgradeHandlers"
    participant Factory as "v9.4.CreateUpgradeHandler"
    participant AffiliateKeeper
    participant Migrator as "Module Migrator"

    App->>UpgradesSetup: register v9.4 Upgrade, SetUpgradeHandler(v9.4.CreateUpgradeHandler)
    UpgradesSetup->>Factory: CreateUpgradeHandler(mm, app.AffiliatesKeeper, configurator)
    activate Factory
    Note right of Factory #bfe3c7: Handler factory receives AffiliateKeeper

    Factory->>AffiliateKeeper: setDefaultAffiliateTiersForSlidingAffiliates(ctx)
    Factory->>AffiliateKeeper: setDefaultAffiliateParameters(ctx)
    Factory->>AffiliateKeeper: migrateAffiliateOverrides(ctx)

    Factory->>Migrator: RunMigrations(ctx, versionMap)
    Migrator-->>Factory: migrations result
    Factory-->>App: Handler complete
    deactivate Factory
Loading

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~45 minutes

Possibly related PRs

Suggested labels

feature:x/affiliates

Suggested reviewers

  • shrenujb
  • tqin7
  • teddyding

Poem

🐰 A v9.4 hop, a tiny cheer,
Tiers and params spring into gear,
Overrides gathered, tidy and neat,
Migrations run, the state's complete—
A carrot-coded upgrade, sweet! 🥕

Pre-merge checks and finishing touches

❌ Failed checks (1 warning)
Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% 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 "Upgrade handler 9.4" is directly related to the main changes in the pull request. The raw summary confirms this PR adds upgrade handler logic for v9.4, wires it into the upgrade system, defines v9.4 upgrade constants, and includes test coverage—all of which align with the title. The title is concise, clear, and specific enough for teammates scanning history to understand this is about implementing the v9.4 upgrade handler. While it could be more specific about affiliate-related changes, the instructions acknowledge that titles don't need to cover every detail.
Description Check ✅ Passed The pull request description follows the required template structure with all three main sections present: Changelist, Test Plan, and Author/Reviewer Checklist. The Changelist provides specific items (affiliate whitelist migrations, tier updates, parameter setup) that align with the raw summary. The Test Plan section, while brief, indicates "Unit tests" are included. The Author/Reviewer Checklist is present with all standard items listed, though none are checked. Overall, the description is mostly complete and matches the repository template, meeting the pass criteria.
✨ Finishing touches
  • 📝 Generate docstrings
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch upgrade_handler_9.4

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

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

Copy link
Contributor

@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: 4

🧹 Nitpick comments (2)
protocol/app/upgrades/v9.4/upgrade_container_test.go (1)

31-36: Consider asserting setup calls succeed (if they return errors).

If Node.SetAffiliate{Tiers,Parameters,Whitelist} return errors, assert them to avoid silent setup failures that mask upgrade issues.

Example (only if methods return errors):

- node.SetAffiliateTiers(constants.PreviousAffiliateTiers)
- node.SetAffiliateParameters(constants.PreviousAffiliateParameters)
- node.SetAffiliateWhitelist(constants.PreviousAffiliateWhitelist)
+ require.NoError(t, node.SetAffiliateTiers(constants.PreviousAffiliateTiers))
+ require.NoError(t, node.SetAffiliateParameters(constants.PreviousAffiliateParameters))
+ require.NoError(t, node.SetAffiliateWhitelist(constants.PreviousAffiliateWhitelist))
protocol/app/upgrades/v9.4/upgrade.go (1)

43-46: Small naming nit: addr → tier.

Loop variable represents a tier, not an address; rename for clarity.

Apply this diff:

- for _, addr := range whitelist.Tiers {
-   overridesList = append(overridesList, addr.Addresses...)
+ for _, tier := range whitelist.Tiers {
+   overridesList = append(overridesList, tier.Addresses...)
 }
📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 33a4abb and b9f1c87.

📒 Files selected for processing (5)
  • protocol/app/upgrades.go (2 hunks)
  • protocol/app/upgrades/v9.4/constants.go (1 hunks)
  • protocol/app/upgrades/v9.4/upgrade.go (1 hunks)
  • protocol/app/upgrades/v9.4/upgrade_container_test.go (2 hunks)
  • protocol/scripts/genesis/sample_pregenesis.json (1 hunks)
🧰 Additional context used
🧬 Code graph analysis (3)
protocol/app/upgrades/v9.4/upgrade.go (2)
protocol/app/upgrades/v9.4/constants.go (3)
  • DefaultAffiliateTiers (52-65)
  • DefaultAffiliateParameters (67-71)
  • UpgradeName (10-10)
protocol/lib/context.go (1)
  • UnwrapSDKContext (31-55)
protocol/app/upgrades/v9.4/upgrade_container_test.go (3)
protocol/testing/containertest/testnet_utils.go (1)
  • UpgradeTestnet (24-66)
protocol/app/upgrades/v9.4/constants.go (5)
  • UpgradeName (10-10)
  • PreviousAffiliateParameters (34-38)
  • PreviousAffiliateWhitelist (40-50)
  • DefaultAffiliateTiers (52-65)
  • DefaultAffiliateParameters (67-71)
protocol/testing/containertest/node.go (1)
  • Node (35-40)
protocol/app/upgrades.go (3)
protocol/app/app.go (1)
  • App (271-376)
protocol/app/upgrades/v9.4/upgrade.go (1)
  • CreateUpgradeHandler (54-69)
protocol/app/upgrades/v9.3/upgrade.go (1)
  • CreateUpgradeHandler (12-22)
⏰ 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-comlink / (comlink) Build and Push
  • GitHub Check: (Mainnet) Build and Push ECS Services / call-build-and-push-vulcan / (vulcan) Build and Push
  • GitHub Check: (Mainnet) Build and Push ECS Services / call-build-and-push-bazooka-lambda / (bazooka) Build and Push Lambda
  • 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-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: (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-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-auxo-lambda / (auxo) Build and Push Lambda
  • 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-comlink / (comlink) 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: check-sample-pregenesis-up-to-date
  • GitHub Check: golangci-lint
  • GitHub Check: benchmark
  • GitHub Check: container-tests
  • GitHub Check: build
  • GitHub Check: build-and-push-mainnet
  • GitHub Check: test-coverage-upload
  • GitHub Check: liveness-test
  • GitHub Check: build-and-push-testnet
  • GitHub Check: unit-end-to-end-and-integration
  • GitHub Check: test-race
  • GitHub Check: Analyze (javascript-typescript)
  • GitHub Check: Analyze (go)
  • GitHub Check: Summary
🔇 Additional comments (6)
protocol/scripts/genesis/sample_pregenesis.json (1)

4105-4105: Confirm intent of app_version change.

app_version moved to "9.0.0-49-g33a4abb2f" from a higher -64 build. If CI or container tests assert versions, this downgrade-like change could be confusing. Please confirm it’s intentional and aligned with the binary used in container tests. If not required, consider leaving it unchanged in sample genesis.

protocol/app/upgrades/v9.4/upgrade_container_test.go (1)

22-24: Good test staging.

Pre-upgrade setup/checks then upgrade then post-checks match the on-chain plan height logic in test utils.

protocol/app/upgrades.go (1)

25-35: Wiring looks correct; duplicate-handler guard is good.

Handler registered for v9.4 with AffiliatesKeeper injection and duplicate check. No issues spotted.

protocol/app/upgrades/v9.4/upgrade.go (2)

24-32: No issues found.

The keeper method UpdateAffiliateParameters at protocol/x/affiliates/keeper/keeper.go:332-335 correctly expects ctx sdk.Context and msg *types.MsgUpdateAffiliateParameters, and the code passes exactly those types in the correct order.


34-52: Original review's core claim is incorrect—this is not a data-loss bug.

The AffiliateOverrides design intentionally differs from AffiliateWhitelist: it stores addresses-only and automatically assigns them the maximum affiliate tier fee share (500,000 ppm) via GetTierForAffiliate. The proto comment explicitly states "These are automatically put at the maximum affiliate tier," and the keeper's deprecated methods confirm this is an intentional design transition. The migration correctly flattens all whitelist tiers into a single override list, which is the intended behavior.

The concern assumes AffiliateOverrides should preserve per-tier fee granularity, but it's explicitly designed not to. All override addresses receive the max tier fee by specification.

Likely an incorrect or invalid review comment.

protocol/app/upgrades/v9.4/constants.go (1)

52-65: Fee-share PPM bounds are valid and within module invariants.

The new TakerFeeSharePpm values (400_000 and 500_000) comply with module constraints. The AffiliatesRevSharePpmCap is defined as 500_000 (50%), and validation enforces that TakerFeeSharePpm must not exceed this cap. Both values are within bounds:

  • 400_000 is comfortably below the cap
  • 500_000 equals the maximum allowed value

The tier structure maintains consistent economics with higher-tier requirements corresponding to higher fee shares.

Comment on lines 19 to 33
PreviousAffilliateTiers = affiliatetypes.AffiliateTiers{
Tiers: []affiliatetypes.AffiliateTiers_Tier{
{
ReqReferredVolumeQuoteQuantums: 0,
ReqStakedWholeCoins: 0,
TakerFeeSharePpm: 200_000,
},
{
ReqReferredVolumeQuoteQuantums: 2_000_000_000_000, // 1M volume
ReqStakedWholeCoins: 0,
TakerFeeSharePpm: 300_000,
},
},
}

Copy link
Contributor

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟡 Minor

Fix typo in variable name: PreviousAffilliateTiers → PreviousAffiliateTiers.

Spelling inconsistency will cause import/usage friction.

Apply this diff:

-	PreviousAffilliateTiers = affiliatetypes.AffiliateTiers{
+	PreviousAffiliateTiers = affiliatetypes.AffiliateTiers{
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
PreviousAffilliateTiers = affiliatetypes.AffiliateTiers{
Tiers: []affiliatetypes.AffiliateTiers_Tier{
{
ReqReferredVolumeQuoteQuantums: 0,
ReqStakedWholeCoins: 0,
TakerFeeSharePpm: 200_000,
},
{
ReqReferredVolumeQuoteQuantums: 2_000_000_000_000, // 1M volume
ReqStakedWholeCoins: 0,
TakerFeeSharePpm: 300_000,
},
},
}
PreviousAffiliateTiers = affiliatetypes.AffiliateTiers{
Tiers: []affiliatetypes.AffiliateTiers_Tier{
{
ReqReferredVolumeQuoteQuantums: 0,
ReqStakedWholeCoins: 0,
TakerFeeSharePpm: 200_000,
},
{
ReqReferredVolumeQuoteQuantums: 2_000_000_000_000, // 1M volume
ReqStakedWholeCoins: 0,
TakerFeeSharePpm: 300_000,
},
},
}
🤖 Prompt for AI Agents
In protocol/app/upgrades/v9.4/constants.go around lines 19 to 33, the exported
variable name is misspelled as PreviousAffilliateTiers; rename it to
PreviousAffiliateTiers everywhere in this file (and update any other files that
import or reference it) so the spelling is consistent, keeping the struct
literal contents unchanged and preserving export visibility; run a project-wide
search/replace for the old identifier and update imports/usages accordingly,
then run `go build`/tests to ensure no references remain.

Comment on lines +35 to +38
Maximum_30DAffiliateRevenuePerReferredUserQuoteQuantums: 1_000_000_000_000, // 10M volume
RefereeMinimumFeeTierIdx: 1,
Maximum_30DAttributableVolumePerReferredUserQuoteQuantums: 1_000_000_000, // 10k volume
}
Copy link
Contributor

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🔴 Critical

🧩 Analysis chain

Verify units and indices in AffiliateParameters.

  • Comment-to-value mapping for “10M volume”/“10k volume” looks off vs the magnitudes used elsewhere (e.g., genesis and asset exponents).
  • RefereeMinimumFeeTierIdx = 2: confirm this index is valid against the configured fee tiers (0/1-based?) to avoid OOB lookups.

Run this to locate definitions and usages:

Also applies to: 67-71


🏁 Script executed:

#!/bin/bash
# Inspect params fields and typical ranges/usages.
rg -n -C3 'AffiliateParameters|RefereeMinimumFeeTierIdx|Maximum_30D(AffiliateRevenue|AttributableVolume)' --type go --type proto

# Cross-check fee tier indices.
rg -n -C3 'MsgUpdatePerpetualFeeParams|feetiers.*tiers|FeeTierIdx' --type go --type proto

Length of output: 59


I need to correct the shell script—rg doesn't recognize "proto" as a valid file type. Let me search more broadly to find the AffiliateParameters definition, fee tier configuration, and verify the indices and magnitudes.


🏁 Script executed:

#!/bin/bash
# Search without --type proto, broader search for Go files
rg -n -C3 'AffiliateParameters|RefereeMinimumFeeTierIdx|Maximum_30D' --type go

# Search for fee tier definitions and indices
rg -n -C3 'FeeTier|feetiers' --type go | head -100

# Also check proto files without specifying type
fd -e proto -x rg -l 'AffiliateParameters|RefereeMinimumFeeTierIdx'

Length of output: 129859


I found critical issues. Let me search for fee tier configuration to verify the index validity.


🏁 Script executed:

#!/bin/bash
# Find fee tier configuration and array definitions
rg -n -A20 'PerpetualFeeParams|Tiers.*\[' --type go | grep -A20 'Tiers'

# Look for fee tier bounds checking or index validation
rg -n 'RefereeMinimumFeeTierIdx.*index|len.*tier|bounds|OOB' --type go

# Check affiliates.proto for field definitions
cat proto/dydxprotocol/affiliates/affiliates.proto 2>/dev/null | grep -A5 -B5 'referee_minimum'

Length of output: 262144


Fix unit comments and fee‑tier index in AffiliateParameters (upgrade constants)

  • protocol/app/upgrades/v9.4/constants.go:35–38 (PreviousAffiliateParameters) — numeric/comment mismatch: 1_000_000_000_000 is commented "10M" but equals 1,000,000 USDC (1M) if quote quantums = 1e6; 1_000_000_000 is commented "10k" but equals 1,000 USDC (1k). Update comments or scale literals to match intended values.
  • RefereeMinimumFeeTierIdx inconsistency: PreviousAffiliateParameters sets 1 while DefaultAffiliateParameters and affiliatetypes.DefaultAffiliateParameters use 2 and tests call index 2 to select tier 3 (fee tiers are 0‑based). Confirm intended starting tier and make PreviousAffiliateParameters match (likely set to 2) or document deliberate divergence.
  • protocol/app/upgrades/v9.4/constants.go:67–71 (DefaultAffiliateParameters) vs protocol/x/affiliates/types/constants.go:34–38 — defaults/magnitudes differ (e.g., attributable volume 10_000_000_000 vs 100_000_000_000_000). Align the upgrade defaults with the module defaults or justify differences.
🤖 Prompt for AI Agents
In protocol/app/upgrades/v9.4/constants.go around lines 35–38, update
PreviousAffiliateParameters so numeric literals and inline comments match
intended magnitudes (either adjust the numeric literals to represent the
commented USDC volumes given quote quantum=1e6, or update comments to reflect
the current literal values); set RefereeMinimumFeeTierIdx to match the rest of
the codebase (change from 1 to 2) so tests and DefaultAffiliateParameters select
the same fee tier; and reconcile DefaultAffiliateParameters at lines ~67–71 with
protocol/x/affiliates/types/constants.go by aligning attributable/affiliate
volume magnitudes or add a clear comment documenting why the upgrade defaults
intentionally differ.

Comment on lines 25 to 26
err = containertest.UpgradeTestnet(nodeAddress, t, node, v_9_4.UpgradeName)
require.NoError(t, err)
Copy link
Contributor

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🔴 Critical

Missing import of v9.4 package causes build failure.

v_9_4.UpgradeName is used but the package isn’t imported.

Apply this diff:

 import (
   "testing"

   "github.com/dydxprotocol/v4-chain/protocol/testing/containertest"
   "github.com/dydxprotocol/v4-chain/protocol/testutil/constants"
   "github.com/stretchr/testify/require"
+  v_9_4 "github.com/dydxprotocol/v4-chain/protocol/app/upgrades/v9.4"
 )
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
err = containertest.UpgradeTestnet(nodeAddress, t, node, v_9_4.UpgradeName)
require.NoError(t, err)
import (
"testing"
"github.com/dydxprotocol/v4-chain/protocol/testing/containertest"
"github.com/dydxprotocol/v4-chain/protocol/testutil/constants"
"github.com/stretchr/testify/require"
v_9_4 "github.com/dydxprotocol/v4-chain/protocol/app/upgrades/v9.4"
)
🤖 Prompt for AI Agents
In protocol/app/upgrades/v9.4/upgrade_container_test.go around lines 25-26, the
test references v_9_4.UpgradeName but the v_9_4 package is not imported, causing
a build failure; fix it by adding the import for the package that defines
UpgradeName with the alias v_9_4 (e.g., add an import line that aliases the
correct module path to v_9_4) so the symbol resolves and the file compiles.

Comment on lines 40 to 45
func postUpgradeChecks(node *containertest.Node, t *testing.T) {
// Check that the affiliate tiers and parameters are set to the default values.
require.Equal(t, constants.DefaultAffiliateTiers, node.GetAffiliateTiers())
require.Equal(t, constants.DefaultAffiliateParameters, node.GetAffiliateParameters())
require.Equal(t, constants.DefaultAffiliateOverrides, node.GetAffiliateOverrides())
}
Copy link
Contributor

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🔴 Critical

🧩 Analysis chain

Double-check expected shape of DefaultAffiliateOverrides.

The migration currently derives overrides from whitelist addresses only. If overrides are expected to encode per-address fee shares, the test expectation may need to reflect that richer structure.


🏁 Script executed:

#!/bin/bash
# Discover AffiliateOverrides schema and node API return shape.
rg -n -C3 'message\s+AffiliateOverrides\b|type\s+AffiliateOverrides\b' --type proto --type go
rg -n -C3 'GetAffiliateOverrides|SetAffiliateOverrides' --type go
rg -n -C3 'DefaultAffiliateOverrides' protocol/testutil/constants

Length of output: 11466



🏁 Script executed:

#!/bin/bash
# Find AffiliateOverrides proto definition
find . -name "*.proto" -type f | head -20

# Search for AffiliateOverrides type definition
rg -n 'type AffiliateOverrides struct|message AffiliateOverrides' --type go -A 10

# Find DefaultAffiliateOverrides constant definition
rg -n 'DefaultAffiliateOverrides\s*=' protocol/testutil/constants -A 3

# Get full GetAffiliateOverridesMap implementation
rg -n 'func.*GetAffiliateOverridesMap' protocol/x/affiliates/keeper/keeper.go -A 15

Length of output: 2496


🏁 Script executed:

#!/bin/bash
# Search for DefaultAffiliateOverrides in constants
rg -n 'DefaultAffiliateOverrides' protocol/testutil -A 5

# Also search more broadly
rg -n 'DefaultAffiliateOverrides' --type go -A 3

Length of output: 198


🏁 Script executed:

#!/bin/bash
# Check contents of constants file
find protocol/testutil -name "*constants*" -type f

# Look at files in protocol/testutil/constants
ls -la protocol/testutil/constants/ 2>/dev/null || echo "Directory not found"

# Search for any Affiliate-related constants
rg -n 'Affiliate' protocol/testutil --type go -B 2 -A 2 | head -50

# Check if DefaultAffiliateOverrides is defined anywhere
grep -r "DefaultAffiliateOverrides\s*:=\|DefaultAffiliateOverrides\s*=" . --include="*.go" 2>/dev/null | head -20

# Look in the test file itself
cat protocol/app/upgrades/v9.4/upgrade_container_test.go | grep -A 10 -B 10 "DefaultAffiliateOverrides"

Length of output: 6421


🏁 Script executed:

#!/bin/bash
# Check imports and top of upgrade_container_test.go
head -50 protocol/app/upgrades/v9.4/upgrade_container_test.go

# Search for 'var DefaultAffiliateOverrides'
rg -n 'var.*DefaultAffiliateOverrides|const.*DefaultAffiliateOverrides' --type go

# Check protocol/testutil/sim_helpers/constants.go for any Affiliate constants
cat protocol/testutil/sim_helpers/constants.go | grep -i affiliate -A 3 -B 1

# List all Go files in protocol/testutil/constants to find where constants are defined
find protocol/testutil/constants -name "*.go" -exec basename {} \; | sort

Length of output: 2174


🏁 Script executed:

#!/bin/bash
# Search genesis.go for DefaultAffiliateOverrides
rg -n 'DefaultAffiliateOverrides' protocol/testutil/constants/genesis.go -B 2 -A 5

# If not found, search all files in constants directory
rg -n 'DefaultAffiliateOverrides' protocol/testutil/constants/ -B 2 -A 5 2>/dev/null | head -40

# Search in sim_helpers as fallback
rg -n 'DefaultAffiliateOverrides' protocol/testutil/sim_helpers/ -B 2 -A 5 2>/dev/null | head -40

Length of output: 0


🏁 Script executed:

#!/bin/bash
# Search entire codebase for DefaultAffiliateOverrides
rg -n 'DefaultAffiliateOverrides' . --type go 2>/dev/null

# Also check if there are any recent additions or TODOs
rg -n 'DefaultAffiliate' protocol/testutil/constants/ --type go

# Look for pattern: Default + Affiliate + something
rg -n 'Default.*Affiliate|Affiliate.*Default' protocol/testutil/constants/ --type go -A 2

Length of output: 142


Define missing DefaultAffiliateOverrides constant in protocol/testutil/constants.

The test at line 44 references constants.DefaultAffiliateOverrides which does not exist anywhere in the codebase. This will cause a compilation or runtime error. The AffiliateOverrides type contains only an Addresses field (a list of strings), so the constant should be defined as an empty or appropriately initialized AffiliateOverrides struct in protocol/testutil/constants.

🤖 Prompt for AI Agents
In protocol/app/upgrades/v9.4/upgrade_container_test.go around lines 40 to 45,
the test references constants.DefaultAffiliateOverrides which is missing; add an
exported constant (or variable) named DefaultAffiliateOverrides to
protocol/testutil/constants with the correct AffiliateOverrides type initialized
with an empty Addresses slice (e.g., AffiliateOverrides{Addresses: []string{}}),
ensure the symbol is exported and the file's package and imports match the rest
of testutil so the test compiles.

Copy link
Contributor

@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: 2

♻️ Duplicate comments (1)
protocol/app/upgrades/v9.4/constants.go (1)

19-42: Fix typo in export: PreviousAffilliateTiers → PreviousAffiliateTiers.

-	PreviousAffilliateTiers = affiliatetypes.AffiliateTiers{
+	PreviousAffiliateTiers = affiliatetypes.AffiliateTiers{

Please update all references across the repo.

#!/bin/bash
# Find and show all occurrences to update.
rg -nP 'PreviousAffilliateTiers|PreviousAffiliateTiers' -C2
🧹 Nitpick comments (5)
protocol/app/upgrades/v9.4/upgrade.go (3)

34-52: Deduplicate whitelist addresses when building overrides; consider clearing legacy whitelist.

Avoid duplicate overrides and stale state. Build a set first; optionally clear/disable old whitelist after migration if the module no longer reads it.

Proposed change:

 func migrateAffiliateOverrides(ctx sdk.Context, affiliateKeeper affiliatekeeper.Keeper) {
@@
-	// Create overrides for all whitelist addresses
-	overrides := affiliatetypes.AffiliateOverrides{}
-	var overridesList []string
-	for _, addr := range whitelist.Tiers {
-		overridesList = append(overridesList, addr.Addresses...)
-	}
-	overrides.Addresses = overridesList
+	// Create overrides for all whitelist addresses (deduped).
+	seen := make(map[string]struct{}, 1024)
+	var list []string
+	for _, tier := range whitelist.Tiers {
+		for _, a := range tier.Addresses {
+			if _, ok := seen[a]; ok {
+				continue
+			}
+			seen[a] = struct{}{}
+			list = append(list, a)
+		}
+	}
+	overrides := affiliatetypes.AffiliateOverrides{Addresses: list}
@@
 	err = affiliateKeeper.SetAffiliateOverrides(ctx, overrides)
 	if err != nil {
 		panic(fmt.Sprintf("failed to set affiliate overrides: %s", err))
 	}
+
+	// Optional: clear or mark legacy whitelist as inactive if no longer used by x/affiliates.
+	// _ = affiliateKeeper.ClearAffiliateWhitelist(ctx) // if available
 }

Please confirm whether x/affiliates still queries the whitelist at runtime. If not, clear it to prevent drift.


16-22: Prefer returning errors over panics inside helpers.

Panics halt the chain; returning wrapped errors through the upgrade handler keeps failure explicit.

Example pattern:

-func setDefaultAffiliateTiersForSlidingAffiliates(ctx sdk.Context, k affiliatekeeper.Keeper) {
-	err := k.UpdateAffiliateTiers(ctx, DefaultAffiliateTiers)
-	if err != nil {
-		panic(fmt.Sprintf("failed to set default affiliate tiers: %s", err))
-	}
-}
+func setDefaultAffiliateTiersForSlidingAffiliates(ctx sdk.Context, k affiliatekeeper.Keeper) error {
+	if err := k.UpdateAffiliateTiers(ctx, DefaultAffiliateTiers); err != nil {
+		return fmt.Errorf("set default affiliate tiers: %w", err)
+	}
+	return nil
+}

Then call and return the error from the handler before RunMigrations.

Also applies to: 24-32


60-62: Include structured fields in the upgrade log.

Add plan name/height to help operators correlate node logs during the cutover.

-		sdkCtx.Logger().Info(fmt.Sprintf("Running %s Upgrade...", UpgradeName))
+		sdkCtx.Logger().Info("running upgrade",
+			"upgrade", UpgradeName,
+			"plan", plan.Name,
+			"height", sdkCtx.BlockHeight(),
+		)
protocol/app/upgrades/v9.4/constants.go (2)

13-18: Operational: mark PR as state-breaking and document migration notes.

This upgrade mutates affiliates’ tiers/params and migrates whitelist → overrides. Label the PR “state-breaking” and add brief operator notes (backup, rollback plan).


77-81: Clarify comment units to improve precision.

The comment "10M volume" is misleading—the value 10_000_000_000_000 represents quote quantums (revenue, not volume). Clarify with quote quantums and optionally note the coin equivalent.

- Maximum_30DAffiliateRevenuePerReferredUserQuoteQuantums:   10_000_000_000_000, // 10M volume
+ Maximum_30DAffiliateRevenuePerReferredUserQuoteQuantums:   10_000_000_000_000, // 10M quote quantums (~$10M if 1e6/coin)

The RefereeMinimumFeeTierIdx: 2 is valid—the default genesis initializes 8+ fee tiers via PromotionalParams(), so index 2 is within bounds.

📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 2853bc6 and 8b25fad.

📒 Files selected for processing (6)
  • protocol/app/upgrades/v9.4/constants.go (1 hunks)
  • protocol/app/upgrades/v9.4/upgrade.go (1 hunks)
  • protocol/app/upgrades/v9.4/upgrade_container_test.go (1 hunks)
  • protocol/testing/version/VERSION_CURRENT (1 hunks)
  • protocol/testing/version/VERSION_FULL_NAME_PREUPGRADE (1 hunks)
  • protocol/testing/version/VERSION_PREUPGRADE (1 hunks)
✅ Files skipped from review due to trivial changes (3)
  • protocol/testing/version/VERSION_CURRENT
  • protocol/testing/version/VERSION_PREUPGRADE
  • protocol/testing/version/VERSION_FULL_NAME_PREUPGRADE
🚧 Files skipped from review as they are similar to previous changes (1)
  • protocol/app/upgrades/v9.4/upgrade_container_test.go
🧰 Additional context used
🧬 Code graph analysis (1)
protocol/app/upgrades/v9.4/upgrade.go (2)
protocol/app/upgrades/v9.4/constants.go (3)
  • DefaultAffiliateTiers (62-75)
  • DefaultAffiliateParameters (77-81)
  • UpgradeName (10-10)
protocol/lib/context.go (1)
  • UnwrapSDKContext (31-55)
⏰ 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: (Public Testnet) Build and Push ECS Services / call-build-and-push-auxo-lambda / (auxo) Build and Push Lambda
  • 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-bazooka-lambda / (bazooka) Build and Push Lambda
  • 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-ender / (ender) 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-ecs-service-roundtable / (roundtable) Build and Push
  • 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-bazooka-lambda / (bazooka) Build and Push Lambda
  • GitHub Check: (Mainnet) Build and Push ECS Services / call-build-and-push-vulcan / (vulcan) 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-ecs-service-socks / (socks) Build and Push
  • GitHub Check: unit-end-to-end-and-integration
  • GitHub Check: test-race
  • 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-ender / (ender) Build and Push
  • GitHub Check: liveness-test
  • GitHub Check: test-coverage-upload
  • GitHub Check: check-sample-pregenesis-up-to-date
  • GitHub Check: build
  • GitHub Check: build-and-push-testnet
  • GitHub Check: benchmark
  • GitHub Check: golangci-lint
  • GitHub Check: container-tests
  • GitHub Check: build-and-push-mainnet
  • GitHub Check: Analyze (javascript-typescript)
  • GitHub Check: Analyze (go)
  • GitHub Check: Summary

Comment on lines +44 to +48
PreviousAffiliateParameters = affiliatetypes.AffiliateParameters{
Maximum_30DAffiliateRevenuePerReferredUserQuoteQuantums: 1_000_000_000_000, // 10M volume
RefereeMinimumFeeTierIdx: 1,
Maximum_30DAttributableVolumePerReferredUserQuoteQuantums: 1_000_000_000, // 10k volume
}
Copy link
Contributor

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🔴 Critical

🧩 Analysis chain

Align numeric literals with comments and units (quote quantums).

“10M volume” vs 1_000_000_000_000 and “10k volume” vs 1_000_000_000 are inconsistent if 1 unit = 1e6 quote quantums.

Pick one:

  • Adjust numbers to match comments:
- Maximum_30DAffiliateRevenuePerReferredUserQuoteQuantums:   1_000_000_000_000, // 10M volume
+ Maximum_30DAffiliateRevenuePerReferredUserQuoteQuantums:  10_000_000_000_000, // 10M
- Maximum_30DAttributableVolumePerReferredUserQuoteQuantums: 1_000_000_000, // 10k volume
+ Maximum_30DAttributableVolumePerReferredUserQuoteQuantums: 10_000_000_000, // 10k
  • Or adjust comments to reflect current values (e.g., “1M”, “1k”).

Also, confirm RefereeMinimumFeeTierIdx = 1 is intentional given Default uses 2.


🏁 Script executed:

#!/bin/bash
# Inspect fee tier indices and ensure 0-based indexing and bounds.
rg -n 'RefereeMinimumFeeTierIdx' -C3
rg -nP '(Fee|Trading).*Tier(s)?\b.*\[' -C2 --type go

Length of output: 17977


Align numeric literals with comments or vice versa in PreviousAffiliateParameters.

The values 1_000_000_000_000 and 1_000_000_000 do not match their comments if quote quantums are scaled (1e6 base units). The new defaults correctly use 10_000_000_000_000 and 10_000_000_000 for "10M" and "10k" respectively. Either scale the previous values up by 10× or adjust comments to "1M" and "1k".

RefereeMinimumFeeTierIdx = 1 in the previous state vs 2 in the new default appears intentional as part of the upgrade migration.

🤖 Prompt for AI Agents
In protocol/app/upgrades/v9.4/constants.go around lines 44 to 48, the numeric
literals in PreviousAffiliateParameters (1_000_000_000_000 and 1_000_000_000) do
not match their "10M" and "10k" comments given quote quantums scale; update
those two values to 10_000_000_000_000 and 10_000_000_000 respectively so the
literals match the comments, and leave RefereeMinimumFeeTierIdx as 1 (no
change).

Comment on lines 63 to 71
// // Set default affiliate tiers and parameters.
// setDefaultAffiliateTiersForSlidingAffiliates(sdkCtx, affiliateKeeper)

// // Set default affiliate parameters.
// setDefaultAffiliateParameters(sdkCtx, affiliateKeeper)

// // Migrate affiliate overrides.
// migrateAffiliateOverrides(sdkCtx, affiliateKeeper)
return mm.RunMigrations(ctx, configurator, vm)
Copy link
Contributor

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🔴 Critical

Migrations are disabled — uncomment to actually apply v9.4 changes.

All three affiliate migration steps are commented, so the upgrade only runs module migrations and never applies tiers/params/overrides.

Apply this diff to enable them:

-		// // Set default affiliate tiers and parameters.
-		// setDefaultAffiliateTiersForSlidingAffiliates(sdkCtx, affiliateKeeper)
-
-		// // Set default affiliate parameters.
-		// setDefaultAffiliateParameters(sdkCtx, affiliateKeeper)
-
-		// // Migrate affiliate overrides.
-		// migrateAffiliateOverrides(sdkCtx, affiliateKeeper)
+		// Set default affiliate tiers.
+		setDefaultAffiliateTiersForSlidingAffiliates(sdkCtx, affiliateKeeper)
+
+		// Set default affiliate parameters.
+		setDefaultAffiliateParameters(sdkCtx, affiliateKeeper)
+
+		// Migrate affiliate overrides.
+		migrateAffiliateOverrides(sdkCtx, affiliateKeeper)
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
// // Set default affiliate tiers and parameters.
// setDefaultAffiliateTiersForSlidingAffiliates(sdkCtx, affiliateKeeper)
// // Set default affiliate parameters.
// setDefaultAffiliateParameters(sdkCtx, affiliateKeeper)
// // Migrate affiliate overrides.
// migrateAffiliateOverrides(sdkCtx, affiliateKeeper)
return mm.RunMigrations(ctx, configurator, vm)
// Set default affiliate tiers.
setDefaultAffiliateTiersForSlidingAffiliates(sdkCtx, affiliateKeeper)
// Set default affiliate parameters.
setDefaultAffiliateParameters(sdkCtx, affiliateKeeper)
// Migrate affiliate overrides.
migrateAffiliateOverrides(sdkCtx, affiliateKeeper)
return mm.RunMigrations(ctx, configurator, vm)
🤖 Prompt for AI Agents
In protocol/app/upgrades/v9.4/upgrade.go around lines 63 to 71, the three
affiliate migration calls are commented out so tiers, parameters, and overrides
never get applied; uncomment the calls to
setDefaultAffiliateTiersForSlidingAffiliates(sdkCtx, affiliateKeeper),
setDefaultAffiliateParameters(sdkCtx, affiliateKeeper), and
migrateAffiliateOverrides(sdkCtx, affiliateKeeper) so they run before returning
mm.RunMigrations(ctx, configurator, vm), and ensure sdkCtx and affiliateKeeper
are in scope and any returned errors from those functions are handled or
propagated as appropriate.

Copy link
Contributor

@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: 1

♻️ Duplicate comments (1)
protocol/app/upgrades/v9.4/upgrade.go (1)

63-71: Migrations now enabled — resolved from prior review.

The three affiliate steps are now invoked before RunMigrations. Good catch and fix.

🧹 Nitpick comments (4)
protocol/app/upgrades/v9.4/upgrade.go (4)

34-52: Deduplicate and sort override addresses for deterministic state.

Current code may insert duplicates and preserves input order. For consensus hygiene, de‑dup and sort before persisting.

Apply:

+	// Build a unique, sorted list of override addresses for deterministic state.
+	addrSet := make(map[string]struct{})
-	// Create overrides for all whitelist addresses
-	overrides := affiliatetypes.AffiliateOverrides{}
-	var overridesList []string
-	for _, addr := range whitelist.Tiers {
-		overridesList = append(overridesList, addr.Addresses...)
-	}
-	overrides.Addresses = overridesList
+	for _, tier := range whitelist.Tiers {
+		for _, a := range tier.Addresses {
+			addrSet[a] = struct{}{}
+		}
+	}
+	overrides := affiliatetypes.AffiliateOverrides{
+		Addresses: make([]string, 0, len(addrSet)),
+	}
+	for a := range addrSet {
+		overrides.Addresses = append(overrides.Addresses, a)
+	}
+	sort.Strings(overrides.Addresses)

Also add the missing import:

 import (
 	"context"
 	"fmt"
+	"sort"

16-22: Prefer error propagation over panics in upgrade path.

Return errors from helper functions and bubble them up from the handler. Keeps crash semantics while preserving clearer failure reporting and stack traces via the upgrade module.

Minimal refactor within this file:

-func setDefaultAffiliateTiersForSlidingAffiliates(ctx sdk.Context, affiliateKeeper affiliatekeeper.Keeper) {
-	err := affiliateKeeper.UpdateAffiliateTiers(ctx, DefaultAffiliateTiers)
-
-	if err != nil {
-		panic(fmt.Sprintf("failed to set default affiliate tiers: %s", err))
-	}
-}
+func setDefaultAffiliateTiersForSlidingAffiliates(ctx sdk.Context, affiliateKeeper affiliatekeeper.Keeper) error {
+	if err := affiliateKeeper.UpdateAffiliateTiers(ctx, DefaultAffiliateTiers); err != nil {
+		return fmt.Errorf("set default affiliate tiers: %w", err)
+	}
+	return nil
+}

-func setDefaultAffiliateParameters(ctx sdk.Context, affiliateKeeper affiliatekeeper.Keeper) {
+func setDefaultAffiliateParameters(ctx sdk.Context, affiliateKeeper affiliatekeeper.Keeper) error {
 	// (keep the out-of-range guard from prior comment)
-	err := affiliateKeeper.UpdateAffiliateParameters(ctx, &affiliatetypes.MsgUpdateAffiliateParameters{
+	if err := affiliateKeeper.UpdateAffiliateParameters(ctx, &affiliatetypes.MsgUpdateAffiliateParameters{
 		AffiliateParameters: DefaultAffiliateParameters,
-	})
-
-	if err != nil {
-		panic(fmt.Sprintf("failed to set default affiliate parameters: %s", err))
-	}
-}
+	}); err != nil {
+		return fmt.Errorf("set default affiliate parameters: %w", err)
+	}
+	return nil
+}

-func migrateAffiliateOverrides(ctx sdk.Context, affiliateKeeper affiliatekeeper.Keeper) {
+func migrateAffiliateOverrides(ctx sdk.Context, affiliateKeeper affiliatekeeper.Keeper) error {
 	// Get all whitelist
 	whitelist, err := affiliateKeeper.GetAffiliateWhitelist(ctx)
 	if err != nil {
-		panic(fmt.Sprintf("failed to get affiliate whitelist: %s", err))
+		return fmt.Errorf("get affiliate whitelist: %w", err)
 	}
 	// (dedup + sort snippet from previous comment)
 	// Update affiliate overrides
-	err = affiliateKeeper.SetAffiliateOverrides(ctx, overrides)
-	if err != nil {
-		panic(fmt.Sprintf("failed to set affiliate overrides: %s", err))
-	}
-}
+	if err := affiliateKeeper.SetAffiliateOverrides(ctx, overrides); err != nil {
+		return fmt.Errorf("set affiliate overrides: %w", err)
+	}
+	return nil
+}

 return func(ctx context.Context, plan upgradetypes.Plan, vm module.VersionMap) (module.VersionMap, error) {
   sdkCtx := lib.UnwrapSDKContext(ctx, "app/upgrades")
-  sdkCtx.Logger().Info(fmt.Sprintf("Running %s Upgrade...", UpgradeName))
+  sdkCtx.Logger().Info("Running Upgrade", "name", UpgradeName)
 
-  // Set default affiliate tiers and parameters.
-  setDefaultAffiliateTiersForSlidingAffiliates(sdkCtx, affiliateKeeper)
+  // Set default affiliate tiers.
+  if err := setDefaultAffiliateTiersForSlidingAffiliates(sdkCtx, affiliateKeeper); err != nil {
+    return nil, err
+  }
 
   // Set default affiliate parameters.
-  setDefaultAffiliateParameters(sdkCtx, affiliateKeeper)
+  if err := setDefaultAffiliateParameters(sdkCtx, affiliateKeeper); err != nil {
+    return nil, err
+  }
 
   // Migrate affiliate overrides.
-  migrateAffiliateOverrides(sdkCtx, affiliateKeeper)
+  if err := migrateAffiliateOverrides(sdkCtx, affiliateKeeper); err != nil {
+    return nil, err
+  }
 
   return mm.RunMigrations(ctx, configurator, vm)
 }

Also applies to: 24-32, 34-52, 59-73


60-62: Use structured logging (avoid fmt.Sprintf).

Minor polish: structured fields aid log querying.

-		sdkCtx.Logger().Info(fmt.Sprintf("Running %s Upgrade...", UpgradeName))
+		sdkCtx.Logger().Info("Running Upgrade", "name", UpgradeName)

63-69: Clarify comment.

Comment says “tiers and parameters” but only tiers are set here.

-		// Set default affiliate tiers and parameters.
+		// Set default affiliate tiers.
📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 8b25fad and 17ff22e.

📒 Files selected for processing (1)
  • protocol/app/upgrades/v9.4/upgrade.go (1 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
protocol/app/upgrades/v9.4/upgrade.go (2)
protocol/app/upgrades/v9.4/constants.go (3)
  • DefaultAffiliateTiers (62-75)
  • DefaultAffiliateParameters (77-81)
  • UpgradeName (10-10)
protocol/lib/context.go (1)
  • UnwrapSDKContext (31-55)
⏰ 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). (26)
  • GitHub Check: unit-end-to-end-and-integration
  • GitHub Check: liveness-test
  • GitHub Check: test-coverage-upload
  • GitHub Check: test-race
  • GitHub Check: golangci-lint
  • GitHub Check: benchmark
  • GitHub Check: check-sample-pregenesis-up-to-date
  • GitHub Check: container-tests
  • 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-roundtable / (roundtable) 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: (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-bazooka-lambda / (bazooka) Build and Push Lambda
  • 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-ecs-service-comlink / (comlink) Build and Push
  • 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-vulcan / (vulcan) Build and Push
  • GitHub Check: (Mainnet) Build and Push ECS Services / call-build-and-push-bazooka-lambda / (bazooka) 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-ender / (ender) 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-ecs-service-socks / (socks) Build and Push
  • GitHub Check: build-and-push-testnet
  • GitHub Check: build-and-push-mainnet
  • GitHub Check: Analyze (go)
  • GitHub Check: Summary
🔇 Additional comments (1)
protocol/app/upgrades/v9.4/upgrade.go (1)

63-72: Verify ordering vs module migrations.

Confirm the affiliates module’s own migrations do not overwrite the tiers/params/overrides you set. If they do, move these calls after mm.RunMigrations.

Proposed ordering (if needed):

-		// Set defaults and migrate overrides...
-		...
-		return mm.RunMigrations(ctx, configurator, vm)
+		vm, err := mm.RunMigrations(ctx, configurator, vm)
+		if err != nil {
+			return nil, err
+		}
+		// Apply affiliate defaults and overrides after module store/params migrations.
+		if err := setDefaultAffiliateTiersForSlidingAffiliates(sdkCtx, affiliateKeeper); err != nil { return nil, err }
+		if err := setDefaultAffiliateParameters(sdkCtx, affiliateKeeper); err != nil { return nil, err }
+		if err := migrateAffiliateOverrides(sdkCtx, affiliateKeeper); err != nil { return nil, err }
+		return vm, nil

Comment on lines +24 to +32
func setDefaultAffiliateParameters(ctx sdk.Context, affiliateKeeper affiliatekeeper.Keeper) {
err := affiliateKeeper.UpdateAffiliateParameters(ctx, &affiliatetypes.MsgUpdateAffiliateParameters{
AffiliateParameters: DefaultAffiliateParameters,
})

if err != nil {
panic(fmt.Sprintf("failed to set default affiliate parameters: %s", err))
}
}
Copy link
Contributor

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🔴 Critical

RefereeMinimumFeeTierIdx likely out of range (will fail upgrade).

Per constants.go, DefaultAffiliateTiers defines 2 tiers (idx 0–1) while DefaultAffiliateParameters sets RefereeMinimumFeeTierIdx = 2. This mismatches and will cause UpdateAffiliateParameters to error, triggering a panic here. Add a guard and fix constants to align (preferred: set idx to 1 or add a third tier). Based on relevant_code_snippets.

Apply this guard locally to fail fast with a clear message:

 func setDefaultAffiliateParameters(ctx sdk.Context, affiliateKeeper affiliatekeeper.Keeper) {
-	err := affiliateKeeper.UpdateAffiliateParameters(ctx, &affiliatetypes.MsgUpdateAffiliateParameters{
+	// Sanity check: ensure referee tier index is within bounds of DefaultAffiliateTiers.
+	if int(DefaultAffiliateParameters.RefereeMinimumFeeTierIdx) >= len(DefaultAffiliateTiers.Tiers) {
+		panic(fmt.Sprintf(
+			"invalid RefereeMinimumFeeTierIdx=%d for %d tiers",
+			DefaultAffiliateParameters.RefereeMinimumFeeTierIdx, len(DefaultAffiliateTiers.Tiers),
+		))
+	}
+	err := affiliateKeeper.UpdateAffiliateParameters(ctx, &affiliatetypes.MsgUpdateAffiliateParameters{
 		AffiliateParameters: DefaultAffiliateParameters,
 	})
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
func setDefaultAffiliateParameters(ctx sdk.Context, affiliateKeeper affiliatekeeper.Keeper) {
err := affiliateKeeper.UpdateAffiliateParameters(ctx, &affiliatetypes.MsgUpdateAffiliateParameters{
AffiliateParameters: DefaultAffiliateParameters,
})
if err != nil {
panic(fmt.Sprintf("failed to set default affiliate parameters: %s", err))
}
}
func setDefaultAffiliateParameters(ctx sdk.Context, affiliateKeeper affiliatekeeper.Keeper) {
// Sanity check: ensure referee tier index is within bounds of DefaultAffiliateTiers.
if int(DefaultAffiliateParameters.RefereeMinimumFeeTierIdx) >= len(DefaultAffiliateTiers.Tiers) {
panic(fmt.Sprintf(
"invalid RefereeMinimumFeeTierIdx=%d for %d tiers",
DefaultAffiliateParameters.RefereeMinimumFeeTierIdx, len(DefaultAffiliateTiers.Tiers),
))
}
err := affiliateKeeper.UpdateAffiliateParameters(ctx, &affiliatetypes.MsgUpdateAffiliateParameters{
AffiliateParameters: DefaultAffiliateParameters,
})
if err != nil {
panic(fmt.Sprintf("failed to set default affiliate parameters: %s", err))
}
}
🤖 Prompt for AI Agents
In protocol/app/upgrades/v9.4/upgrade.go around lines 24 to 32, the code blindly
applies DefaultAffiliateParameters which sets RefereeMinimumFeeTierIdx = 2 while
DefaultAffiliateTiers only defines two tiers (indices 0–1), causing
UpdateAffiliateParameters to fail and panic; fix this by making the constants
consistent (preferred: change RefereeMinimumFeeTierIdx to 1 or add a third tier
to DefaultAffiliateTiers) and add a pre-check here that validates
RefereeMinimumFeeTierIdx is within the bounds of len(DefaultAffiliateTiers) and
returns/panics with a clear message if out of range before calling
affiliateKeeper.UpdateAffiliateParameters.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Development

Successfully merging this pull request may close these issues.

1 participant