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

Update volume limits #182

Merged
merged 2 commits into from
Aug 17, 2023
Merged

Update volume limits #182

merged 2 commits into from
Aug 17, 2023

Conversation

nagakingg
Copy link
Collaborator

@nagakingg nagakingg commented Jul 28, 2023

To do:

  • need to scale total volumes correctly; this may depend on a subgraph update

Description

Previously, all volume limits were in dollars. This approximation worked for v1 pools where all assets were worth ~1 dollar, but will not work for v2.

Coingecko volumes are now returned in the base currency for each pair, and volume limits for trades in the opposite direction are computed using the current price at each tick in the volume limited arb pipeline.

Hygiene checklist

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

Cute Animal Picture

image

@nagakingg
Copy link
Collaborator Author

These changes do not affect the simple pipeline, but do affect the volume limited arbitrage pipeline. There, the volume limits are altered slightly for v1 pools because the limits are no longer in USD, but in units of the actual currency being traded.

Testing with 3pool, the results are similar enough to be visually indistinguishable, but not numerically identical (as expected).

Old volume limiter:
image

New volume limiter:
image

Full plot html files in this zip:
3pool_volume_limiter_comparison.zip

@nagakingg nagakingg requested review from chanhosuh and removed request for chanhosuh July 28, 2023 20:14
@nagakingg nagakingg force-pushed the vol-mult-update branch 3 times, most recently from d3346e8 to 995aa7b Compare August 16, 2023 22:49
@github-actions
Copy link
Contributor

github-actions bot commented Aug 16, 2023

Coverage report

The coverage rate went from 75.23% to 75.04% ⬇️
The branch rate is 61%.

69.23% of new lines are covered.

Diff Coverage details (click to unfold)

curvesim/network/coingecko.py

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

curvesim/pipelines/vol_limited_arb/strategy.py

20% of new lines are covered (45.83% of the complete file).
Missing lines: 32, 37, 38, 41

@nagakingg nagakingg force-pushed the vol-mult-update branch 2 times, most recently from 801a72e to d27b016 Compare August 17, 2023 00:17
@nagakingg nagakingg force-pushed the vol-mult-update branch 2 times, most recently from 779f3af to df79810 Compare August 17, 2023 02:15
@nagakingg nagakingg marked this pull request as ready for review August 17, 2023 02:36
nagakingg and others added 2 commits August 16, 2023 22:14
Updates volume limiting to support
multiple asset denominations.

Previously, all volume limits were in
dollars. This approximation worked for
v1 pools where all assets were worth
~1 dollar, but will not work for v2.

Coingecko volumes are now returned
in the base currency for each pair,
and volume limits for trades in the
opposite direction are computed using
the current price at each tick in the
volume limited arb pipeline.
@chanhosuh chanhosuh merged commit 6a89084 into main Aug 17, 2023
1 check passed
@chanhosuh chanhosuh deleted the vol-mult-update branch August 17, 2023 12:46
@chanhosuh chanhosuh added this to the Cryptopool Sims milestone Aug 17, 2023
@chanhosuh chanhosuh mentioned this pull request Aug 17, 2023
14 tasks
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.

vol_mult param in autosim no longer works as expected
2 participants