[go: nahoru, domu]

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

Add possibility to use rate oracle as provider of rates to calculate … #3492

Merged
merged 14 commits into from
Jun 9, 2021

Conversation

aarmoa
Copy link
Contributor
@aarmoa aarmoa commented Jun 3, 2021

Before submitting this PR, please make sure:

  • Your code builds clean without any errors or warnings
  • You are using approved title ("feat/", "fix/", "docs/", "refactor/")

A description of the changes proposed in the pull request:

Add possibility in amm-arb strategy to use the rate oracle to get rates required to calculate arbitrage proposals profitability.
Added a new object called FixedRateSource, polymorphic with RateOracle, that is used when the user configures the strategy to use a provided quotes conversion rate instead of the oracle.

https://app.zenhub.com/workspaces/hummingbot-5f340755c5e17300167a8f30/issues/coinalpha/hummingbot/3216
https://github.com/CoinAlpha/hummingbot/issues/3216

@aarmoa aarmoa linked an issue Jun 3, 2021 that may be closed by this pull request
@aarmoa aarmoa requested a review from Nullably June 3, 2021 21:10
@aarmoa aarmoa mentioned this pull request Jun 3, 2021
@dennisocana dennisocana requested a review from a team June 4, 2021 14:20
Nullably
Nullably previously approved these changes Jun 7, 2021
Copy link
Contributor
@Nullably Nullably left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@RC-13
Copy link
Contributor
RC-13 commented Jun 7, 2021
  • The current profitability shows 0.00% both when doing a test on KOVAN and Mainnet:
    image
    amm-rate oracle

  • WETH markets don't start due to the rate not recognize:
    image

@aarmoa
Copy link
Contributor Author
aarmoa commented Jun 8, 2021

@RC-13, would it be possible to have more information about the scenario configured to trade LUNA-UST in Terra and LUNA-USDT in Binance?
If we consider the order amount is always the same, then the profitability is calculated as following:

P_buyT_sellB = ((sell_price_B * rate_USDT-UST) - buy_price_T) / buy_price_T
P_buyT_sellB = ((6.423 * 1.01) - 6.60768371) / 6.60768371
P_buyT_sellB = (6.48723 - 6.60768371) / 6.60768371
P_buyT_sellB = -0.01822933955

P_buyB_sellT = ((sell_price_T * rate_UST-USDT) - buy_price_B) / buy_price_B
P_buyB_sellT = ((6.25507632 * 0.9900990099) - 6.427) / 6.427
P_buyB_sellT = (6.193144871 - 6.427) / 6.427
P_buyB_sellT = -0.03638635891

In both arbitrage proposals the profitability is negative, and for sure less than the minimum configured profitability. I am not considering the fees since I don't know their value, but I don't think they will impact too much in the result.
Which side did you expect to be profitable?

@aarmoa
Copy link
Contributor Author
aarmoa commented Jun 8, 2021

I have committed a change to make the scenario of uniswap (WETH-DAI) and binance (ETH-USDC) work. The profit calculation logic assumes that always the arbitrage sides have the same base token, and avoids looking for a base tokens conversion rate. In the example case the rate oracle would never find the conversion rate between WETH and ETH.

@aarmoa
Copy link
Contributor Author
aarmoa commented Jun 8, 2021

I found an issue in the logic that displays the profitability status description. The issue was causing the client to display profitability 0.0% for any arbitrage proposal (because the profitability calculation was not being called with the correct rate source). It should now be fixed.

… workflow causing the validation process to hang
@aarmoa aarmoa requested a review from Nullably June 8, 2021 21:11
@RC-13
Copy link
Contributor
RC-13 commented Jun 9, 2021

Thanks, @aarmoa for looking into the reported issue. Confirming that the reported issue regarding the profitability not getting displayed accordingly has been resolved on the latest commit.:
image
Also, the base conversion rate is now checked, if the market base is different:
image

  • Minor cosmetic issue: after initially starting the strategy, then changing the market of one of the connectors. The conversion rate message still displays the first market conversion rate rather than the new one. On this screenshot, the initial markets are LUNA-UST and LUNA-USDT then changed to LUNA-USDT and LUNA-BUSD.
    image
    Though if you run the status command, it will show the right conversion rate for the new market (BUSD-UST)
  • For the WETH market issue, I will open a separate ticket.

RC-13
RC-13 previously approved these changes Jun 9, 2021
Copy link
Contributor
@Nullably Nullably left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@dennisocana dennisocana merged commit f32d3a5 into development Jun 9, 2021
@dennisocana dennisocana deleted the feat/add_rate_oracle_to_amm_arb branch June 9, 2021 05:21
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.

Add rate oracle to amm-arb
4 participants