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

Exclude too-small trades from optimizer #279

Merged
merged 2 commits into from
Nov 6, 2023
Merged

Exclude too-small trades from optimizer #279

merged 2 commits into from
Nov 6, 2023

Conversation

nagakingg
Copy link
Collaborator

@nagakingg nagakingg commented Oct 30, 2023

Description

Fixes #223 -- trades are excluded from optimization in multipair_optimal_arbitrage if their volume limit is <= pool.get_min_trade_size().

Additionally, trades are excluded from optimization in get_arb_trades if the optimal trade size is less than pool.get_min_trade_size() (i.e., the minimum trade size would cause the pool price to become less than the target market price). In this case, a trade in the right direction, but with size 0 is returned. This prevents similar errors as those discussed in #223, and prevents trades that are smaller than a pool's min trade size, but within the single-trade optimization in get_arb_trades.

Important change with downstream effects:
Previously, if no profitable arb was available for a particular pair, get_arb_trades would return a trade with size 0 in whatever direction the coin symbols were listed in. E.g., if market prices were indexed as (ETH, BTC), it would return a 0 trade with ETH as coin in and BTC as coin out. This had downstream effects for multipair_optimal_arbitrage (because only trades in that direction were allowed in the multi-trade optimization) and subsequent price error calculations (because price error was always computed in that arbitrary direction).

With these changes, if no profitable arb is available (give the minimum trade size), a trade with size 0 in the more profitable direction is returned. This allows 'multipair_optimal_arbitrage' to make trades in that direction if they become available after previous trades in each optimizer iteration, and causes price error to always be computed in the direction of the best possible trade.

Additional changes:

  • Added ArbTrade class to templates.trader. It is a Trade object with a target price field, and a method to change the amount_in.
  • Price errors are now returned as a dict with format (base, quote): error

Hygiene checklist

  • Changelog entry
  • Everything public has a Numpy-style docstring
    (modules, public functions, classes, and public methods)
  • Commit history is cleaned-up with minor changes squashed together
    and descriptive commit messages following Tim Pope's style

Cute Animal Picture

image

@github-actions
Copy link
Contributor

Coverage report

The coverage rate went from 76.03% to 75.64% ⬇️
The branch rate is 62%.

29.62% of new lines are covered.

Diff Coverage details (click to unfold)

curvesim/metrics/metrics.py

100% of new lines are covered (83.76% of the complete file).

curvesim/pipelines/common/init.py

100% of new lines are covered (91.3% of the complete file).

curvesim/pipelines/simple/trader.py

100% of new lines are covered (100% of the complete file).

curvesim/pipelines/vol_limited_arb/strategy.py

0% of new lines are covered (42.85% of the complete file).
Missing lines: 44, 46

curvesim/pipelines/vol_limited_arb/trader.py

8.77% of new lines are covered (12.17% of the complete file).
Missing lines: 76, 77, 79, 80, 81, 83, 84, 86, 88, 89, 92, 94, 96, 99, 100, 101, 117, 122, 124, 126, 129, 130, 131, 133, 142, 143, 144, 145, 146, 147, 149, 150, 152, 154, 159, 160, 168, 169, 171, 172, 174, 204, 205, 206, 207, 208, 210, 211, 212, 213, 214, 216

curvesim/templates/trader.py

70% of new lines are covered (84.12% of the complete file).
Missing lines: 31, 32, 46

@nagakingg nagakingg force-pushed the exclude-trades branch 2 times, most recently from 5895bb2 to 2ac26b1 Compare October 31, 2023 13:16
@nagakingg
Copy link
Collaborator Author

The simple pipeline CI is unaffected by these changes, except for the price_error values, which differ slightly due to the new computation.

For the volume limited pipeline, the results change slightly. Qualitatively, they look pretty similar but perhaps slightly denoised.

3pool on current main branch:

3pool_old_opt

3pool on this branch:

3pool_new_opt

@nagakingg
Copy link
Collaborator Author

The comparison is simlar for TriCRV, except that the price errors come out quite noisy. Since the price errors are reduced using the new computation, this may reflect a noise floor. But I wonder if its better to record price errors as percect deviation from each target price. Currently they are absolute, which probably makes less sense for v2 pools.

TriCRV on current main branch:

triCRV_old_opt

TriCRV on this branch:

triCRV_new_opt

@nagakingg
Copy link
Collaborator Author

I wonder if its better to record price errors as percect deviation from each target price

Indeed, dividing each price error by the target price cleans up the price error plots considerably:

image

@nagakingg nagakingg marked this pull request as ready for review November 2, 2023 19:18
- Exclude trades smaller than pool's minimum trade size from optimization
  in get_arb_trades and multipair_optimal arbitrage
- Scale price errors by target price
- Return price errors as dicts with trading pair as key
- Add ArbTrade class to templates.trader
@chanhosuh chanhosuh merged commit 103b828 into main Nov 6, 2023
1 check passed
@chanhosuh chanhosuh deleted the exclude-trades branch November 6, 2023 17:50
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.

Exclude trading pair from multipair_optimal_arbitrage if volume limit is 0
2 participants