-
Notifications
You must be signed in to change notification settings - Fork 423
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Filter out additional incorrect TVL pools #3888
base: stage
Are you sure you want to change the base?
Conversation
@JohnnyWyles is attempting to deploy a commit to the OsmoLabs Team on Vercel. A member of the Team first needs to authorize it. |
WalkthroughThe changes in this pull request focus on the Changes
Possibly related PRs
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 (
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
🧹 Outside diff range and nitpick comments (1)
packages/server/src/queries/complex/pools/index.ts (1)
22-22
: Consider a more maintainable approach for managing filtered pools.While hardcoding pool IDs works for now, it might become difficult to maintain as the list grows. Consider implementing a more flexible solution, such as:
- Storing filtered pool IDs in a configuration file.
- Implementing a dynamic filtering mechanism based on pool properties.
This would make it easier to update the list of filtered pools in the future without modifying the code.
Would you like assistance in designing a more scalable solution for managing filtered pools?
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (1)
- packages/server/src/queries/complex/pools/index.ts (1 hunks)
🧰 Additional context used
🔇 Additional comments (1)
packages/server/src/queries/complex/pools/index.ts (1)
22-22
: LGTM, but needs testing and documentation.The addition of pool IDs "2217" and "2218" to
FILTERABLE_IDS
aligns with the PR objective of filtering out additional incorrect TVL pools. However, please ensure the following:
- Test these changes locally before merging.
- Add a comment explaining why these specific pool IDs are being filtered.
To verify the impact of this change, please run the following script:
This will help ensure that filtering these pools doesn't have unintended consequences elsewhere in the codebase.
✅ Verification successful
To continue verifying the impact of adding pool IDs "2217" and "2218" to
FILTERABLE_IDS
, please run the following script:
To accurately verify references to pool IDs "2217" and "2218" without false positives, please run the following script:
Verification Successful: No other references to pool IDs "2217" and "2218" found.
The addition of pool IDs "2217" and "2218" to
FILTERABLE_IDS
has been validated with no unintended references in other parts of the codebase.Please ensure the following before merging:
- Test these changes locally to confirm they behave as expected.
- Add a comment explaining why these specific pool IDs are being filtered.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Check for any references to the newly filtered pool IDs echo "Searching for references to pool IDs 2217 and 2218:" rg --type typescript --type javascript '2217|2218'Length of output: 200
Script:
# #!/bin/bash # Description: Search for references to pool IDs 2217 and 2218 in TypeScript and JavaScript files echo "Searching for references to pool IDs 2217 and 2218 in .ts, .tsx, .js, and .jsx files:" rg '2217|2218' -g '*.ts' -g '*.tsx' -g '*.js' -g '*.jsx'Length of output: 2182
Script:
# #!/bin/bash # Description: Search for exact references to pool IDs 2217 and 2218 in TypeScript and JavaScript files echo "Searching for exact references to pool IDs 2217 and 2218 in .ts, .tsx, .js, and .jsx files:" rg '\b2217\b|\b2218\b' -g '*.ts' -g '*.tsx' -g '*.js' -g '*.jsx'Length of output: 375
I strongly protest, there is no reason to block these pools. Moreover, introducing a filter at this level is a crime. And there is an ongoing discussion on this topic https://forum.osmosis.zone/t/pool-2159/3228/4 |
What is the purpose of the change:
Similar to #3864
This pool was noticed to be filtered and so 2 new ones were made with the same singlesided tactic, skewing displayed TVL.
Linear Task
Brief Changelog
Adds 2 pools to list
Testing and Verifying
This change has not been tested locally