Skip to content
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

Convergence issues with overlapping strategies #594

Merged
merged 4 commits into from
May 7, 2024

Conversation

sklbancor
Copy link
Collaborator

@sklbancor sklbancor commented May 2, 2024

MUST BE MERGE INTO #606 ASAP

Overlapping strategies are designed to recreate Uniswap v3 like strategies on Carbon, ie ranges that trade bidirectional taking a small fee. Unsurprisingly -- in hindsight -- those strategies behave as badly within the Optimizer as other strategies with fees would (see #588). There is a reason that we ignore fees in the Optimizer and add them back only in the fine tuning step.

High level the solution is actually pretty easy: remove the effective fee component from overlapping strategies. The current PR is designed to achieve that. However, it is a bit hacky, and once we have time we need to also take into account a few considerations spelled out in the next comment

@sklbancor
Copy link
Collaborator Author

The current strategy dealing with effective fee removal for overlapping strategies works, but there are some issues that need to be address in the long run

  • The optimal solution depends on the size of the fee (or bid/offer; the two are the same): the smaller the fee, the worse the Optimizer is affected, and the less impact on the overall solution it has to remove it; the bigger the fee, the more benign it is for the Optimizer (eg, 10% fee is definitely not a problem) and the worse it gets when we remove it; so we need to work on the threshold between removing the fee or not
  • Where those adjustments are to be made is not clear; a priori that is a numerical issue that is better understood on the Optimizer / CPC side (and definitely not on the Node side); however, there are admin issues to consider as well, eg regarding CIDs
  • In the long run, we should probably provide a Carbon strategy constructor that encapsulates the current Carbon order constructor because then the CPC has the information it needs to understand whether or not to make adjustments (at the moment it just sees different orders and it does not connect them); we discussed the issues around CIDs of this approach with Nick, and possibly the move to a CID / Sub CID approach
  • Relatedly we noticed that the current CPC code creates curves that are optimized for the MargPOptimizer (notably, enforcing minimum curvature; the new code modifying strategies in place would fall into the same category); whilst this kinda makes sense -- we really only use MargPOptimizer -- this is not clean: the curves should be created as they are, and modified when reading into the Optimizer; however, this is a major change and it may not in practice be worth the effort.

fastlane_bot/helpers/poolandtokens.py Outdated Show resolved Hide resolved
@sklbancor
Copy link
Collaborator Author

@NIXBNT just making sure we agree that this is your PR; I just opened it for you

@sklbancor sklbancor changed the base branch from add_sei_bp to add_sei May 6, 2024 13:41
@sklbancor sklbancor force-pushed the convergence_overlap branch from aec37e4 to ff834ab Compare May 6, 2024 13:44
@sklbancor sklbancor marked this pull request as ready for review May 6, 2024 13:44
fastlane_bot/utils.py Outdated Show resolved Hide resolved
fastlane_bot/utils.py Outdated Show resolved Hide resolved
fastlane_bot/helpers/poolandtokens.py Show resolved Hide resolved
@barakman barakman self-requested a review May 7, 2024 07:39
@barakman barakman merged commit 9a640a8 into add_sei May 7, 2024
1 check passed
@barakman barakman deleted the convergence_overlap branch May 7, 2024 12:38
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants