zelos-alpha / demeter

Better backtest toolkit for Uniswap v3 and Aave.
https://medium.com/zelos-research
MIT License
32 stars 7 forks source link

Granting too much fees when opening up multiple high liquidity positions #4

Closed mbartn closed 1 year ago

mbartn commented 1 year ago

The function demeter.uniswap.market.UniLpMarket.__update_fee updates LP fees for each position opened.

However, if position_liquidity > pool_liquidity, the position's share is 1 so it receives all the fees generated by the pool in the current timestamp_index. This leads to a bug where for n positions opened, broker earns n * all_fees_generated_in_current_timestamp_index

I think when calculating share, liquidity of all brokker's positions should be considered as well https://github.com/zelos-alpha/demeter/blob/b91c228d208d71001ad5c0f59c3074be7c132a5f/demeter/uniswap/core.py#L92-L96

turfT commented 1 year ago

I think it is caused by its assumption. During backtesting, your liquidity will not add to the market's total liquidity. "If" of Share calculation is defensive code.

turfT commented 1 year ago

We will take your case into consideration, and maybe fix it in the next version. Or, you can help us fix it, and make a PR.

32ethers commented 1 year ago

This bug has fixed and will be available in next release.