feat: Set max n_coins on stable swap. added explanation of ann #181
Conversation
WalkthroughThe changes add a new constant Changes
Sequence Diagram(s)sequenceDiagram
participant U as User
participant CP as create_pool
participant V as Validator
U->>CP: Call create_pool(coins)
CP->>V: Validate coins.length (≥2 and ≤ MAX_N_COINS)
V-->>CP: Return validation result
CP-->>U: Return pool or error
Suggested reviewers
Poem
🪧 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: 0
🔭 Outside diff range comments (1)
precompile/modules/initia_stdlib/sources/stableswap.move (1)
1490-1570: 💡 Verification agent❓ Verification inconclusive
Add test cases for MAX_N_COINS validation.
The end-to-end test doesn't verify the new maximum coin limit. Consider adding test cases to verify:
- Creation of a pool with maximum allowed coins (5)
- Failure when attempting to create a pool with more than 5 coins
🏁 Script executed:
#!/bin/bash # Check if there are any test cases verifying MAX_N_COINS rg -A 5 "MAX_N_COINS" "**/*test*.move"Length of output: 93
Add tests for MAX_N_COINS boundary conditions
The current end-to-end test does not cover the new MAX_N_COINS validation. To ensure robust coverage, please add test cases that:
- Verify that creating a pool with exactly 5 coins (the maximum allowed) succeeds.
- Confirm that attempting to create a pool with more than 5 coins fails as expected.
Also, manually verify whether any tests exist in other locations that already address this, as our automated search did not return any results.
🧹 Nitpick comments (1)
precompile/modules/initia_stdlib/sources/stableswap.move (1)
29-31: Enhance the documentation for better clarity.While the formula explanation is good, consider adding more details about:
- What 'A' represents (amplification factor's role in the pool)
- What 'n' represents (number of coins)
- Why this calculation is important for the pool's behavior
/// ANN, calculated as A * n ** n * A_PRECISION, where A is the amplification factor. /// All `ann` values remain consistent with this formula. +/// - A: Amplification factor that controls the pool's price curve +/// - n: Number of coins in the pool +/// - A_PRECISION: Scaling factor (100) to maintain precision +/// Higher values of A result in more stable prices around the 1:1 ratio.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
precompile/modules/initia_stdlib/sources/stableswap.move(3 hunks)
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: Rust libmovevm
🔇 Additional comments (2)
precompile/modules/initia_stdlib/sources/stableswap.move (2)
19-19: LGTM! Good addition of MAX_N_COINS constant.The constant helps prevent potential overflow issues by limiting the number of coins in a pool.
568-570: LGTM! Good validation of pool size limits.The assertion correctly enforces both minimum (2) and maximum (5) limits on the number of coins in a pool.
Description
MAX_N_COINSfor stable swap to prevent overflow.Closes: #XXXX
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...
!in the type prefix if API or client breaking changeReviewers 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...
Summary by CodeRabbit
New Features
Documentation