tradingstrategy-ai / trade-executor

A Python framework for managing positions and trades in DeFi
https://tradingstrategy.ai
Other
107 stars 29 forks source link

Confirm and fix Uniswap v2 clone fee structure #129

Closed miohtama closed 1 year ago

miohtama commented 1 year ago

Both entry and exit estimates are off by 0.05%. Because this is constant across all trades, it is unlikely to be natural slippage.

image

AlexTheLion123 commented 1 year ago

@miohtama So I would change this method to accept exchange_slug or equivalent instead oflp_fees parameter, and then get the fee from the routing_data for the relevant exchange?

https://github.com/tradingstrategy-ai/trade-executor/blob/ec628553aa97e5842803fee4d296501ff2f733b4/tradeexecutor/state/state.py#L153-L173

miohtama commented 1 year ago

This is the factory

def uniswap_v2_live_pricing_factory(
        execution_model: ExecutionModel,
        universe: TradingStrategyUniverse,
        routing_model: UniswapV2SimpleRoutingModel) -> UniswapV2LivePricing:

    assert isinstance(universe, TradingStrategyUniverse)
    assert isinstance(execution_model, (UniswapV2ExecutionModelVersion0, UniswapV2ExecutionModel)), f"Execution model not compatible with this execution model. Received {execution_model}"
    assert isinstance(routing_model, UniswapV2SimpleRoutingModel), f"This pricing method only works with Uniswap routing model, we received {routing_model}"

    web3 = execution_model.web3
    return UniswapV2LivePricing(
        web3,
        universe.universe.pairs,
        routing_model)

To fix this properly

How does this sound like?

AlexTheLion123 commented 1 year ago

@miohtama See PR #136. One question, if routing_model is passed as a parameter to UniswapV2LivePricing, do we need to pass the fee as a parameter as well? Because we can get the fee from the routing_model