search

CoinAlpha / gateway-api

Apache License 2.0
47 stars 25 forks source link

(feat) fixes #2557: makes ETH routes non Balancer specific #10

Closed fengtality closed 3 years ago

fengtality commented 3 years ago

Fixes https://github.com/CoinAlpha/hummingbot/issues/2557

To make these functions non Balancer specific, this PR adds a spenderAddress parameter to /eth/approve and /eth/allowances, so the Balancer connector in the client needs to be changed to pass in the Balancer exchangeProxy address (mainnet: 0x3E66B66Fd1d0b02fDa6C811Da9E0547970DB2f21).

I think the Balancer exchangeProxy address should be a global parameter that we set to the current mainnet address. If users want to test on Kovan, they are responsible for changing it.

Nullably commented 3 years ago

The ExchangeProxy (or Balancer contract address) is now stored at 2 locations, HB global config and Gateway .env. I think we should keep it at just one location to avoid confusion.
Would it be better to create an approve endpoint in balancer which redirects to approve in eth and supplies it with spenderAddress? And HB client doesn't need to store the spender address.

fengtality commented 3 years ago

OK, that makes sense. I'll make the change tmrw.

fengtality commented 3 years ago

@Nullably I added approve and allowances endpoints to the Balancer routes.

I spent a lot of time trying to figure out a more elegant way using redirects or proxy requests to avoid duplicating the code, but failed to make it work.

fengtality commented 3 years ago

@vic-en has an alternate method of calling approve and allowances that prevents code duplication in the Balancer routes - he's adding to the Uniswap gateway, and will modify this PR too