feat: use automatic routing #531

KirillDogadin-std · 2022-11-10T14:27:51Z

Closes #527

Checklist:

issue number linked above after pound (#)
- replace "Closes " with "Contributes to" or other if this PR does not close the issue
issue checkboxes are all addressed
manually checked my feature / not applicable
wrote tests / not applicable
attached screenshots / not applicable

KirillDogadin-std · 2022-11-10T14:34:12Z

I've added some codechanges that already indicate what code adjustment i'm intending to implement. Please go through them and point out some decisions that are incorrect/in fact harmful.

E.g. https://github.com/sidestream-tech/unified-auctions-ui/pull/531/files#diff-c6b4bdf1b82a84a00b56df1404f49c064a303b5919e55e1335c7afecaab1a7edR12 seems questionable.

core/src/calleeFunctions/helpers/uniswapAutoRouter.ts

core/src/calleeFunctions/helpers/uniswapV2.ts

core/src/calleeFunctions/CurveLpTokenUniv3Callee.ts

core/src/calleeFunctions/helpers/uniswapV3.ts

core/src/calleeFunctions/index.ts

core/src/types.ts

KirillDogadin-std · 2022-11-14T12:25:20Z

Encountering error when route is called

TypeError: Cannot read properties of undefined (reading 'sortsBefore')
    at V3PoolProvider.getPoolAddress (pool-provider.js?276b:86:1)
    at CachingV3PoolProvider.getPoolAddress (caching-pool-provider.js?9cf4:67:1)
    at CachingV3PoolProvider.getPools (caching-pool-provider.js?9cf4:30:1)
    at getHighestLiquidityV3NativePool (gas-factory-helpers.js?fa4f:32:1)
    at V3HeuristicGasModelFactory.buildGasModel (v3-heuristic-gas-model.js?385b:110:1)
    at async Promise.all (:3000/index 0)
    at async AlphaRouter.route (alpha-router.js?d7e0:329:1)

unified-auctions-ui/core/src/calleeFunctions/helpers/uniswapAutoRouter.ts

Line 32 in 81859b0

const route = await router.route(

KirillDogadin-std · 2022-11-15T12:22:34Z

The call of fetchAutoRouteInformation on production env works, for local setup it does not:

Possible reason: chain id is actually hardcoded :/ . SO when the router is created on main, the chain id corresponds to the actual one. On local setup it does not. Even on main branch the error appears (in the dashboard). So i would guess that setting the actual chain id should work (via fetching it with the existing method).
But in this case the error is thrown:

Error: No address for Uniswap Multicall Contract on chain id: 1337

@valiafetisov do you have the instant idea on how to solve this? If not, i then go investigate

valiafetisov · 2022-11-15T12:39:38Z

The call of fetchAutoRouteInformation on production env works, for local setup it does not:

If by "local setup" you mean "hardhat fork", then yes. It seems to be hardcoded:

unified-auctions-ui/core/src/calleeFunctions/helpers/uniswapAutoRouter.ts

Line 24 in 695d43d

const router = new AlphaRouter({ chainId: 1, provider });

Maybe making it dynamic will already solve the error?

KirillDogadin-std · 2022-11-15T12:41:46Z

So i would guess that setting the actual chain id should work (via fetching it with the existing method).
But in this case the error is thrown:

This paragraph was intended to let know that I've already tried your suggestion above. See the error is that is thrown ('No address for Uniswap Multicall...') in the same comment.

valiafetisov · 2022-11-15T13:10:14Z

This paragraph was intended

Sorry, I thought that reinstalling the dependencies solved the original error #531 (comment)

No address for Uniswap Multicall Contract on chain id: 1337

This error is expected, since 1337 is a chain id that exist only for us. Uniswap SDK doesn't have an idea what is this chain id, as they have to unfortunately still hardcode some staff per chain id in the SDK (eg contract addresses). So for the hardhat fork we would still need to provide chain id that the hardhat is forking, not the one it is calling itself. This will solve the above problem and will bring us back to the initial:

TypeError: Cannot read properties of undefined (reading 'sortsBefore')

Which according to a quick search originates from https://stackoverflow.com/questions/64674034/uniswap-typeerror-cannot-read-property-sortsbefore-of-undefined invalid Token instance creation. We create it via getUniswapTokenBySymbol where decimalChainId is passed derived from network instead of using the same as passed to the AlphaRouter – so the mismatch is formed

KirillDogadin-std · 2022-11-15T16:55:03Z

Currently some tests started to fail after the 'eth-a' collateral was extended with the automatic routing config.
One of the tests is Fetches not liquidatable vault and it goes into the process of extracting the route automatically. it hangs during AlphaRouter.route() call in getUniswapAutoRoute function.

See commit 87dfced which pinpoints the place.
To reproduce you can run npm run test -- ./test/vault-test.ts (the very first test fails / times out for me)

Adjusting the mocha test timeout to 100 seconds instead of 40s did not solve the problem.

valiafetisov · 2022-11-16T09:03:30Z

It's certainly the timeout. Dashboard on the hardhat first time took a very long time for me to load. So a lot of requests are made to get all pools, I assume. Not sure how to go about it, but to fix tests (until we have cache) we might want to skip this fetching

valiafetisov · 2022-12-01T16:58:41Z

tests also fail for crveth with Dog/not-unsafe

Can you please cherry-pick 925b3d9 to temporary fix the tests?

valiafetisov · 2022-12-01T17:00:40Z

core/src/constants/COLLATERALS.ts

+            'Uniswap V3 Autorouter': {
+                callee: 'UniswapV3Callee',
+                automaticRouter: true,
+            },


core/src/calleeFunctions/index.ts

valiafetisov

Tests fixed, code approved

LukSteib

UI Test on goerli looks good ✅
Tested keeper on goerli and executed ETH auction via swap (including auth txs) ✅
Tested keeper on hardhat fork with ETH-A simulation running
- authorisation steps successfully executed ✅
- for swap tx I get the following error in the console (see below)

ℹ collateral keeper: auction "ETH-A:840": attempting swap execution                                                       
 ERROR  collateral keeper: unexpected error: Cannot read properties of undefined (reading 'addresses')

Raising this as I am not able to reproduce it on main branch.

KirillDogadin-std · 2022-12-05T09:31:34Z

Current bugs seem to be related to:

feat: use automatic routing #531 (comment) not being fulfilled properly. At some cases the pools are not pregenerated apparently.
routeToPool logic does not cover the edgecase where the route is empty array. Therefore for this case empty pools will be generated

LukSteib · 2022-12-05T09:37:21Z

I need more context / info in order to properly decide how to proceed here:

Is the problem of not properly pregenerated pools only related to hardhat fork?
Do we know how to tackle the problems described above and only need more time to address?

KirillDogadin-std · 2022-12-05T09:40:48Z

Is the problem of not properly pregenerated pools only related to hardhat fork?

no, this is a code logic issue.

Do we know how to tackle the problems described above and only need more time to address?

(1) requires me to put more thinking in to figure out how to wire up things properly, @valiafetisov might come up with the fix faster.
(2) is simplier, it's just an algorithmic issue.

LukSteib · 2022-12-05T09:58:47Z

(1) requires me to put more thinking in to figure out how to wire up things properly, @valiafetisov might come up with the fix faster.

Ok, then I'd suggest to have @valiafetisov look into it in order to get this PR over the last mile and proceed with release.

valiafetisov · 2022-12-05T10:02:04Z

Yes, it seems like routeToPool will produce invalid pool based on exchanges[marketId].route. It should include starting token into the array. Eg if ETH-A['Uniswap V3'].route is [], the pool should include ETH, DAI, but currently (as far as I understood) only is DAI. Overall, the getRouteSteps + routeToPool seems to be overly complicated

KirillDogadin-std · 2022-12-05T12:21:22Z

I've fixed (2), the side effect of this seems to be advance for (1) - the error is changed

now with

Hardhat fork
ETH-A auction simulation
choosing autorouter

The error is "Error: VM Exception while processing transaction: reverted with reason string 'Too little received'" from JSON-RPC

I was able to use the old Uniswap V3 router to execute swap

valiafetisov · 2022-12-05T17:21:40Z

core/src/calleeFunctions/index.ts

+    const route =
+        'route' in calleeConfig ? calleeConfig.route : await getCalleeAutoRoute(network, collateral, marketId, amount);
+    return await routeToPool(network, route, collateral.symbol);


As pointed out in the issue several times:

We should expect that intermediate tokens might be tokens that are not part of the collateral config

we CAN NOT assume that the route provided from the auto router will ONLY contain symbols that are known to the maker. Therefore we CAN NOT convert automatic route to pool. Instead, the information provided by the AlphaRouter should contain enough information to build a pool object: addresses, symbols and fees.

valiafetisov · 2022-12-05T17:53:31Z

core/src/calleeFunctions/UniswapV3Callee.ts

+    const pools = await routeToPool(network, route, collateral.symbol, fees);
+    const daiAmount = await convertCollateralToDaiUsingPool(
+        network,
+        collateral.symbol,
+        marketId,
+        collateralAmount,
+        pools
+    );


Maybe a better explanation would've been: previously we expected that pools (/routes) are static. Now we should receive and store pools together with prices (because re-requesting price via auto-router can change the pool) – so if the pool is generated, it have to be stored in the auction together with the price (unless the price is not an auction price, but just a reference price)

valiafetisov · 2022-12-05T18:48:27Z

Was able to execute transaction using auto router on a fork (at least that's how it looks)

But two problems outlined above still there:

We're regenerating pools in a different moment than when we fetch prices (so the price displayed and the route displayed are different and requires two requests)
There will be error during pool generation in case auto router returns symbol that is not present in the chainlog (not sure how it will be handled, hard to simulate)

LukSteib · 2022-12-05T19:34:18Z

Next round of tests:

UI Test on goerli looks good ✅
- was able to execute a tx via auto router see: https://goerli.etherscan.io/tx/0xe479025dea52a8f5c87c427de00bbd645631e3380836ed6bcd892d81362ae24a -> net profit indicator was also very accurate
Tested UI on hardhat fork and was able to execute ETH-A Auction via auto router ✅
Tested keeper on hardhat fork with ETH-A simulation running
- authorisation steps successfully executed ✅
- swap tx ✅

LukSteib · 2022-12-05T19:44:06Z

But two problems outlined above still there:

We're regenerating pools in a different moment than when we fetch prices (so the price displayed and the route displayed are different and requires two requests)
There will be error during pool generation in case auto router returns symbol that is not present in the chainlog (not sure how it will be handled, hard to simulate)

We will tackle these problems in a dedicated follow up issue and proceed with the merge of this PR for now.

feat: use automatic routing

06c2ff7

KirillDogadin-std mentioned this pull request Nov 10, 2022

Extend multi-callee with auto router #527

Closed

11 tasks

valiafetisov reviewed Nov 10, 2022

View reviewed changes

KirillDogadin-std added 13 commits November 11, 2022 12:48

improvements

1a13592

lint

ca21a62

rename

85d58e3

pools generated, route ignored

9820bcc

rm usesess block of code

b69041d

add dai to pools

6c8481e

lint

5d812be

rm useless arg

1ee2f0e

rm useless comment

6bab52a

imporve

a776b77

refactor: only v3 is affected

6a7d437

fix prev functionality

6169244

latest

81859b0

KirillDogadin-std added 6 commits November 15, 2022 15:00

explicitly defined the chain id

c354a73

improve chain id arg

d441c42

lint

c5c6c01

Merge remote-tracking branch 'origin/main' into multi-callee

26c5d5e

Use the quote if awailable

70c954f

Demo of too long test

87dfced

valiafetisov reviewed Dec 1, 2022

View reviewed changes

valiafetisov added 2 commits December 1, 2022 18:04

fix _getMarketPrice export

7a0ccfc

hardcode block number in the past

eed3c37

valiafetisov previously approved these changes Dec 1, 2022

View reviewed changes

Merge branch 'main' into multi-callee

76f647d

LukSteib reviewed Dec 2, 2022

View reviewed changes

fix: route generation

f2861a3

KirillDogadin-std dismissed valiafetisov’s stale review via f2861a3 December 5, 2022 12:07

valiafetisov added 4 commits December 5, 2022 16:50

Merge branch 'main' into multi-callee

923522f

fix console error from upstream

c250c8f

reorder simulations

4ff3d92

fix MarketPriceSelection enable button

1a887f5

valiafetisov reviewed Dec 5, 2022

View reviewed changes

properly pass fees to the get pools

cc8dfb7

LukSteib approved these changes Dec 5, 2022

View reviewed changes

LukSteib merged commit 0c51938 into main Dec 5, 2022

LukSteib deleted the multi-callee branch December 5, 2022 19:45

LukSteib mentioned this pull request Dec 6, 2022

Improve uni v3 auto router implementation #558

Closed

2 tasks

KirillDogadin-std mentioned this pull request Dec 7, 2022

patch: Improve autorouter #561

Merged

5 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat: use automatic routing #531

feat: use automatic routing #531

KirillDogadin-std commented Nov 10, 2022 •

edited by valiafetisov

Loading

KirillDogadin-std commented Nov 10, 2022

KirillDogadin-std commented Nov 14, 2022

KirillDogadin-std commented Nov 15, 2022

valiafetisov commented Nov 15, 2022

KirillDogadin-std commented Nov 15, 2022 •

edited

Loading

valiafetisov commented Nov 15, 2022 •

edited

Loading

KirillDogadin-std commented Nov 15, 2022

valiafetisov commented Nov 16, 2022 •

edited

Loading

valiafetisov commented Dec 1, 2022 •

edited

Loading

valiafetisov Dec 1, 2022

valiafetisov left a comment

LukSteib left a comment •

edited

Loading

KirillDogadin-std commented Dec 5, 2022 •

edited

Loading

LukSteib commented Dec 5, 2022

KirillDogadin-std commented Dec 5, 2022

LukSteib commented Dec 5, 2022

valiafetisov commented Dec 5, 2022

KirillDogadin-std commented Dec 5, 2022

valiafetisov Dec 5, 2022 •

edited

Loading

valiafetisov Dec 5, 2022

valiafetisov commented Dec 5, 2022 •

edited

Loading

LukSteib commented Dec 5, 2022 •

edited

Loading

LukSteib commented Dec 5, 2022

feat: use automatic routing #531

feat: use automatic routing #531

Conversation

KirillDogadin-std commented Nov 10, 2022 • edited by valiafetisov Loading

KirillDogadin-std commented Nov 10, 2022

KirillDogadin-std commented Nov 14, 2022

KirillDogadin-std commented Nov 15, 2022

valiafetisov commented Nov 15, 2022

KirillDogadin-std commented Nov 15, 2022 • edited Loading

valiafetisov commented Nov 15, 2022 • edited Loading

KirillDogadin-std commented Nov 15, 2022

valiafetisov commented Nov 16, 2022 • edited Loading

valiafetisov commented Dec 1, 2022 • edited Loading

valiafetisov Dec 1, 2022

Choose a reason for hiding this comment

valiafetisov left a comment

Choose a reason for hiding this comment

LukSteib left a comment • edited Loading

Choose a reason for hiding this comment

KirillDogadin-std commented Dec 5, 2022 • edited Loading

LukSteib commented Dec 5, 2022

KirillDogadin-std commented Dec 5, 2022

LukSteib commented Dec 5, 2022

valiafetisov commented Dec 5, 2022

KirillDogadin-std commented Dec 5, 2022

valiafetisov Dec 5, 2022 • edited Loading

Choose a reason for hiding this comment

valiafetisov Dec 5, 2022

Choose a reason for hiding this comment

valiafetisov commented Dec 5, 2022 • edited Loading

LukSteib commented Dec 5, 2022 • edited Loading

LukSteib commented Dec 5, 2022

KirillDogadin-std commented Nov 10, 2022 •

edited by valiafetisov

Loading

KirillDogadin-std commented Nov 15, 2022 •

edited

Loading

valiafetisov commented Nov 15, 2022 •

edited

Loading

valiafetisov commented Nov 16, 2022 •

edited

Loading

valiafetisov commented Dec 1, 2022 •

edited

Loading

LukSteib left a comment •

edited

Loading

KirillDogadin-std commented Dec 5, 2022 •

edited

Loading

valiafetisov Dec 5, 2022 •

edited

Loading

valiafetisov commented Dec 5, 2022 •

edited

Loading

LukSteib commented Dec 5, 2022 •

edited

Loading