Update large cap params in genesis #2431
Conversation
WalkthroughThe pull request introduces significant updates to the Changes
Possibly related PRs
Suggested labels
Suggested reviewers
Poem
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
🧹 Outside diff range and nitpick comments (7)
protocol/testing/genesis.sh (7)
Line range hint
140-145: Inconsistency in 'maintenance_fraction_ppm' for 'Large-Cap' Liquidity TierThe
maintenance_fraction_ppmis set to600,000ppm (60%), but the comment states it's "60% of IM". Given that theinitial_margin_ppmis20,000ppm (2%), 60% of IM would be12,000ppm (1.2%). Please verify ifmaintenance_fraction_ppmshould be12,000ppm instead of600,000ppm.
Line range hint
158-159: Mismatch in 'maintenance_fraction_ppm' for 'Small-Cap' Liquidity TierThe
maintenance_fraction_ppmis set to500,000ppm (50%), but the comment indicates "50% of IM". With aninitial_margin_ppmof100,000ppm (10%), 50% of IM should be50,000ppm (5%). Consider correcting themaintenance_fraction_ppmvalue or updating the comment for accuracy.
Line range hint
176-177: Discrepancy in 'maintenance_fraction_ppm' for 'Long-Tail' Liquidity TierThe
maintenance_fraction_ppmis500,000ppm (50%), yet the comment notes "50% of IM". Asinitial_margin_ppmis200,000ppm (20%), 50% of IM equates to100,000ppm (10%). Please confirm whethermaintenance_fraction_ppmshould be100,000ppm.
Line range hint
200-201: Clarification on 'maintenance_fraction_ppm' in 'Isolated' Liquidity TierThe
maintenance_fraction_ppmis set to600,000ppm (60%), with a comment stating "60% of IM". Sinceinitial_margin_ppmis50,000ppm (5%), 60% of IM is30,000ppm (3%). Verify if themaintenance_fraction_ppmshould be30,000ppm instead of600,000ppm.
Line range hint
34-37: Ensure Proper Quoting of Variables in 'edit_genesis' FunctionIn the
edit_genesisfunction, variables like${2},${3},${4}, and${5}may contain spaces or special characters. To prevent word splitting or globbing issues, consider quoting these variables when using them.
Line range hint
218-223: Refactoring Suggestion for Exchange Configuration UpdatesThe function
update_all_markets_with_fixed_price_exchangeuses a loop to update each market's exchange configuration individually. Since the operation is similar for all markets, consider abstracting the repeateddaselcommands into a separate function or using arrays to handle the updates more efficiently.
Line range hint
689-691: Potential for Unquoted Variables in LoopsIn the loops within functions like
update_genesis_use_test_exchange, variables such as$market_idxare used without quotes. While numeric variables are generally safe, it's good practice to quote variables to prevent issues if the script is modified in the future.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (2)
- protocol/scripts/genesis/sample_pregenesis.json (1 hunks)
- protocol/testing/genesis.sh (1 hunks)
🔇 Additional comments (2)
protocol/testing/genesis.sh (2)
Line range hint
738-754: Variable 'vault_owner_address' May Be UnsetIn the
edit_genesisfunction, when settingvault_owner_address, ifINPUT_TEST_ACCOUNTS[0]is empty or unset, the default address is used. Ensure thatINPUT_TEST_ACCOUNTSis properly initialized or handle the case where it might be empty to prevent unexpected behavior.
Line range hint
226-228: Possible Missing Error Handling for 'jq' CommandThe
NUM_MARKETSvariable is set usingjq. Ifjqfails or is not installed, the script may proceed with an empty or incorrect value, leading to errors. Consider adding error handling to ensurejqexecutes successfully before proceeding.
| @@ -2985,7 +2985,7 @@ | |||
| { | |||
| "id": 0, | |||
| "impact_notional": 10000000000, | |||
| "initial_margin_ppm": 50000, | |||
| "initial_margin_ppm": 20000, | |||
There was a problem hiding this comment.
Significant reduction in initial margin for Large-Cap tier
The initial_margin_ppm for the "Large-Cap" liquidity tier (id: 0) has been reduced from 50000 to 20000. This represents a 60% decrease in the initial margin requirement for this tier.
Implications:
- Increased leverage: Users will be able to open larger positions with the same amount of collateral.
- Potential risk: Lower margin requirements could lead to increased volatility and higher liquidation risks.
- Market competitiveness: This change might make the platform more attractive to traders seeking higher leverage.
Please ensure that:
- This change aligns with the overall risk management strategy.
- Other risk parameters (e.g., liquidation thresholds) have been adjusted accordingly.
- The impact on the system's stability has been thoroughly assessed.
- Clear communication is prepared for users about this change in margin requirements.
Consider adding a comment in the configuration file explaining the rationale behind this significant change to aid future maintenance and audits.
Changelist
[Describe or list the changes made in this PR]
Test 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
Bug Fixes
Documentation