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 trading pair from multipair_optimal_arbitrage if volume limit is 0 #223

Closed
nagakingg opened this issue Sep 6, 2023 · 1 comment · Fixed by #279
Closed

Exclude trading pair from multipair_optimal_arbitrage if volume limit is 0 #223

nagakingg opened this issue Sep 6, 2023 · 1 comment · Fixed by #279
Assignees

Comments

@nagakingg
Copy link
Collaborator

nagakingg commented Sep 6, 2023

Version Information

All curvesim versions for all Python versions and OSs

What's your issue about?

In the volume limited arbitrage pipeline, when a trading pair is limited to 0 volume, the trading size for that pair is still included as a free parameter to estimate in multipair_optimal_arbitrage. This can cause scipy.optimize.least_squares to hang or raise a ValueError "'x' is not within the trust region".

How can it be fixed?

Any trade with a size limit less than a pool's minimum trade size will never be executed during optimization, so they can be excluded from the least_squares optimization. When there are no eligible trades, optimization can/should be skipped.

The most simple adjustment is this, but:

  • this whole section can be made cleaner
  • the price error output needs to be adjusted so it includes price errors for all pairs even if they are excluded from optimization
def multipair_optimal_arbitrage(pool, prices, limits):
    init_trades = get_arb_trades(pool, prices)

    limited_init_trades = []
    for t in init_trades:
        size, pair, price_target = t
        limit = int(limits[pair] * 10**18)
        t = min(size, limit), pair, price_target, 0, limit + 1
        if t[0] > pool.get_min_trade_size(pair[0]):  # Only include trade if it is possible for it to be executed
            limited_init_trades.append(t)

    if not limited_init_trades: # Skip optimization if no eligible trades
        return [], [], []

[...]

To do this more cleanly, I propose:

  1. Reorganize the above proposed code:

    • Make get_arb_trades return a list of Trade objects (currently it uses amorphous tuples)
    • Rewrite for clarity using the Trade object attribute names
    • Encapsulate all into a function that returns a list of dicts that can be passed as kwargs to least_squares
  2. Introduce a get_price_error() function that combines the price errors returned by least_squares with price errors for any excluded pairs

    • Currently, succesful optimization returns res.fun (i.e., post_trade_price_error_multi for the optimized trades, taken from least_squares results object)
    • When the optimizer fails, we return post_trade_price_error_multi for a series of null trades:
      errors = post_trade_price_error_multi([0] * len(sizes), price_targets, coins)
    • get_price_error(err.fun, skipped_pairs) could concat optimizer results (if any) with the null trades method (if needed) to handle all cases (i.e., all pairs traded, some pairs traded, no pairs traded, optimizer error)
@chanhosuh
Copy link
Member

re: Trade objects, maybe we can extend to ArbTrade which has a price_target attribute.

I would agree we should cleanup the function. It is rather lengthy, which is why there are explanatory comments. These shouldn't really be necessary if there are smaller functions properly named with helpful signatures.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants