skip-mev / connect

A general purpose oracle leveraging ABCI++
Other
80 stars 50 forks source link

fix: uniswap unable to handle more than 10 tickers #797

Closed aljo242 closed 3 weeks ago

aljo242 commented 1 month ago

Closes CON-1849

Uniswap has been found to only support 10 tickers. Any ticker amount larger will cause the entire batch fetching to fail.

What I have found from investigating geth is that nodes are configured with a batchItemLimit, but default values seem to be large enough for anything we realistically would do.

Regardless, from running the test I added and playing with the batchSize of our requests, all sizes > 10 will fail (any size LTE 10 works) for a variety of endpoints.

As it is written, the batch calls are serial, but we could parallelize this. Until we are seeing realistic performance boundaries, I don't think we should prioritize this (and complicate the code).

Leaving the test and test file for demonstration. Will remove the file and test once moving from draft. The test file is generated for dydx main net, filtered for only uniswap markets

codecov[bot] commented 1 month ago

Codecov Report

Attention: Patch coverage is 80.48780% with 8 lines in your changes missing coverage. Please review.

Project coverage is 58.04%. Comparing base (f4be9c5) to head (2e0cc5a).

Files with missing lines Patch % Lines
providers/apis/defi/ethmulticlient/client.go 0.00% 4 Missing :warning:
providers/apis/defi/raydium/client.go 0.00% 4 Missing :warning:
Additional details and impacted files ```diff @@ Coverage Diff @@ ## main #797 +/- ## ========================================== + Coverage 57.34% 58.04% +0.70% ========================================== Files 213 214 +1 Lines 14823 14844 +21 ========================================== + Hits 8500 8616 +116 + Misses 5709 5602 -107 - Partials 614 626 +12 ```

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.