Update DefaultFundingPpm for PML markets
#2610
Conversation
WalkthroughThe changes in this pull request primarily involve modifications to the Changes
Possibly related PRs
Suggested labels
Suggested reviewers
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (2)
protocol/testing/genesis.sh(34 hunks)protocol/x/listing/types/constants.go(1 hunks)
🔇 Additional comments (2)
protocol/x/listing/types/constants.go (1)
6-6: LGTM - Funding rate calculation is correct
The new value of 100 PPM (0.01%) per 8 hours equals 0.125 basis points per hour, which is correctly documented in the comment. This introduces a reasonable baseline funding rate for PML markets.
protocol/testing/genesis.sh (1)
221-221: Confirmation of 'default_funding_ppm' updates to '100' for all perpetual markets
The default_funding_ppm parameter has been updated to 100 across all perpetual markets, including the TEST-USD market. This change aligns with the PR objective to modify this parameter for PML markets and ensures consistency throughout the genesis configuration.
Also applies to: 231-231, 241-241, 251-251, 261-261, 271-271, 281-281, 291-291, 301-301, 311-311, 321-321, 331-331, 341-341, 351-351, 361-361, 371-371, 381-381, 391-391, 401-401, 411-411, 421-421, 431-431, 441-441, 451-451, 461-461, 471-471, 481-481, 491-491, 501-501, 511-511, 521-521, 531-531, 541-541, 2273-2273
| @@ -3,7 +3,7 @@ package types | |||
| const ( | |||
| MinPriceChangePpm_LongTail uint32 = 800 | |||
|
|
|||
| DefaultFundingPpm = 0 | |||
| DefaultFundingPpm = 100 // 1bps per 8 hour or 0.125bps per hour | |||
There was a problem hiding this comment.
💡 Codebase verification
Based on the search results, I can now generate the final response:
Funding rate change requires upgrade handler for state migration
The change from 0 to 100 PPM for DefaultFundingPpm is indeed a state-breaking change that requires careful handling during upgrades. This is evidenced by:
- All existing markets in genesis files (mainnet, testnet, preupgrade) have
default_funding_ppmset to 0 - The change affects all perpetual markets in the protocol
- The genesis scripts and testing configurations consistently use 0 as the default value
🔗 Analysis chain
Verify state-breaking implications of funding rate change
The change from 0 to 100 PPM introduces funding payments where there were none before. This could be a state-breaking change that requires special handling during upgrades.
Consider documenting the upgrade process and any necessary state migrations in the PR description.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Search for upgrade handlers or migration logic related to funding parameters
rg -A 5 "UpgradeHandler|MigrateState.*funding"
# Look for any existing markets that might be affected
rg -A 5 "default_funding_ppm"
Length of output: 92398
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Outside diff range and nitpick comments (1)
protocol/scripts/genesis/sample_pregenesis.json (1)
Line range hint
3034-3386: Review the uniform funding rate across different market tiersThe change sets
default_funding_ppmto 100 for all perpetual markets, regardless of their liquidity tier. Consider if this uniform approach is appropriate given the different risk profiles:
- Tier 0 (Large-Cap): BTC-USD, ETH-USD
- Tier 1 (Small-Cap): Most altcoins
- Tier 2 (Long-Tail): MKR-USD, COMP-USD, APE-USD, BLUR-USD, LDO-USD, SEI-USD
Consider if the funding rate should be tiered based on:
- Market liquidity
- Historical volatility
- Risk profile of the asset
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (1)
protocol/scripts/genesis/sample_pregenesis.json(34 hunks)
🔇 Additional comments (2)
protocol/scripts/genesis/sample_pregenesis.json (2)
Line range hint 3034-3919: Market parameters are well-configured for different asset characteristics
The configuration shows appropriate customization for each market:
- Atomic resolution matches asset precision requirements (e.g., -16 for PEPE, -5 for BTC)
- Price change minimums are calibrated to asset volatility
- Liquidity tiers properly segment assets by market cap and risk
3919-3919: Verify the implications of major version upgrade
The app version has been updated from 5.2.1 to 7.0.0-dev0, which is a major version bump. This could indicate breaking changes.
Changelist
Also updated
DefaultFundingPpmfor testnetsTest Plan
[Describe how this PR was tested (if applicable)]
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
New Features
TEST-USDto improve trading conditions.default_funding_ppmfor multiple markets from0to100, impacting operational dynamics.Bug Fixes
DefaultFundingPpmconstant to reflect accurate funding rates, ensuring clarity for users.