-
Notifications
You must be signed in to change notification settings - Fork 197
fix: dupe position in RUNE and asym rune #10255
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
Conversation
📝 WalkthroughWalkthroughUpdated runeBalances logic in LpType.tsx to stop returning sym display amounts when only a sym position exists. It now returns AsymSide.Rune display amounts only if hasAsymRunePosition is true; otherwise it falls back to newPosition. Asset and sym computations unchanged; no exported API changes. Changes
Sequence Diagram(s)sequenceDiagram
participant UI as LP Type Component
participant State as Positions State
participant Display as DisplayAmounts
UI->>State: Check hasAsymRunePosition
alt hasAsymRunePosition == true
UI->>Display: displayAmounts(AsymSide.Rune)
Display-->>UI: Rune asym amounts
else
UI->>UI: use newPosition path
end
Estimated code review effort🎯 2 (Simple) | ⏱️ ~8 minutes Assessment against linked issues
Assessment against linked issues: Out-of-scope changes(no out-of-scope functional changes identified) Suggested reviewers
Poem
📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (1)
💤 Files with no reviewable changes (1)
⏰ 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). (1)
✨ Finishing Touches
🧪 Generate unit tests
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. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. 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.
This is not a bug, though maybe we can figure out how to be more clear. This was done intentionally as when you start with a symmetric position, adding asymmetric rune does not create a new position and the state is essentially shared between Sym/Asym Rune. This is exactly what is reflected in the UI, but you need to understand the why I suppose.
|
It is still possible and valid though. You can start with a sym position and add either sym or asym rune to the same position. That is why they show the same amounts because they are the same. We can remove the amounts, but then that would be confusing in a different way (ie. "How much is in the position I am adding to?"). Maybe like a link icon between the two? This is the only scenario of a shared position btw. |
Think a link would be a very simple and valid solution! The fact that it's the only case like that, and that it only works when adding more asym RUNE (but not asset, which would be a new position) definitely seems like a case of us needing to disambiguate things to users. Although counterpoint would be that we may not want to expose that capability because of how confusing it is. @twblack88 @reallybeard ping ^^ let's disambiguate this guy! |
|
Meh LP |
Description
Fixes asym position appearing as a dupe in asym RUNE card
Issue (if applicable)
closes #10254
Risk
None
Testing
Engineering
Operations
Screenshots (if applicable)
Summary by CodeRabbit