feat: UniswapV3 Provider #311

davidterpay · 2024-04-04T22:39:16Z

closes: BLO-1054

Uniswap V3 Provider implementation

Note: around ~2800 LOC from this PR is from code-generation, much more is testing. I recommend reading over the readme before reviewing

codecov · 2024-04-04T22:58:46Z

Codecov Report

Attention: Patch coverage is 75.57604% with 53 lines in your changes are missing coverage. Please review.

Project coverage is 56.93%. Comparing base (35c352e) to head (5c02f55).

Files	Patch %	Lines
pkg/math/testutils.go	0.00%	22 Missing ⚠️
providers/apis/defi/uniswapv3/fetcher.go	87.59%	8 Missing and 8 partials ⚠️
providers/factories/oracle/api.go	14.28%	6 Missing ⚠️
providers/apis/defi/uniswapv3/client.go	0.00%	4 Missing ⚠️
providers/base/api/handlers/api_query_handler.go	70.00%	3 Missing ⚠️
providers/apis/defi/uniswapv3/utils.go	84.61%	1 Missing and 1 partial ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##             main     #311      +/-   ##
==========================================
- Coverage   62.55%   56.93%   -5.62%     
==========================================
  Files         221      227       +6     
  Lines        9771    11006    +1235     
==========================================
+ Hits         6112     6266     +154     
- Misses       3103     4179    +1076     
- Partials      556      561       +5

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

davidterpay · 2024-04-04T23:39:29Z

providers/apis/uniswapv3/utils.go

+	Interval:         2000 * time.Millisecond,
+	ReconnectTimeout: 2000 * time.Millisecond,
+	MaxQueries:       1,
+	URL:              "https://eth.public-rpc.com/",


This RPC is probably fine to have displayed on our main, can be a good default for people.

pkg/math/math_test.go

pkg/math/math.go

pkg/math/test_utils.go

providers/apis/uniswapv3/fetcher.go

providers/apis/uniswapv3/fetcher_test.go

providers/apis/uniswapv3/utils.go

aljo242 · 2024-04-07T19:52:34Z

we got conflicts :(

davidterpay · 2024-04-08T14:29:54Z

btw our lint is marking files i did not change with errors

nivasan1 · 2024-04-08T19:01:26Z

providers/apis/defi/uniswapv3/client.go

+// GoEthereumClient is a go-ethereum client implementation using the go-ethereum RPC
+// library.
+type GoEthereumClientImpl struct {
+	client *rpc.Client


Why do we need to wrap?

nivasan1

2 small comments. My follow up thoughts:

Do we need to be able to correlate requests among multiple rpc-providers (like solana)
As needed for raydium, we'll need to have some way to verify that the ProviderMetadata per ticker unmarshals correctly into the expected json-data structure. Ideally, we'd be able to do this on market-map validation

nivasan1 · 2024-04-08T19:04:43Z

providers/apis/defi/uniswapv3/utils.go

+}
+
+// MustToJSON converts the pool configuration to JSON.
+func (pc *PoolConfig) MustToJSON() string {


Where is this used?

nivasan1 · 2024-04-08T19:07:32Z

providers/apis/defi/uniswapv3/fetcher.go

+	}
+
+	// Parse the result from the batch call for each ticker.
+	for i, ticker := range tickers {


Is there a way to guarantee the order of responses from batch-json RPC requests?

the client should return things in the same order, but we can probably resort just for sanity before using the responses.

Eric-Warehime · 2024-04-08T19:00:04Z

README.md

-
-## Future Work
-
-The oracle side car is a combination of the oracle and provider packages. This is being moved to a [separate repository](https://github.com/skip-mev/slinky-sidecar).


Definitely want this removed 👍 wasn't aware this was in here.

Eric-Warehime · 2024-04-08T19:00:27Z

README.md

@@ -98,21 +98,3 @@ To test these numbers yourself, spin up the the oracle server following the inst
 * [Data Provider Queries](./providers/base/metrics/README.md#usage): Provides general insight into how often price feeds are updated by status (success/failure), provider (binance, coinbase, etc.), price feed (BTC/USD, ETH/USD), and provider type (api/websocket).
 * [Websocket Performance Queries](./providers/base/websocket/metrics/README.md#usage): Provides insight into how often websocket providers are successfully updating their data. This is a combination of metrics related to the underlying connection as well as the data handler which is responsible for processing the data received from the Websocket connection.
 * [API Performance Queries](./providers/base/api/metrics/README.md#usage): Provides insight into how often API providers are successfully updating their data.
-
-## DEFI


You need to sync w/ @nivasan1 because I think he's just added all of this and you're removing it now.

Eric-Warehime · 2024-04-08T19:01:16Z

go.mod

-	github.com/gagliardetto/solana-go v1.9.3
+	github.com/ethereum/go-ethereum v1.13.14
+	github.com/gagliardetto/binary v0.8.0
+	github.com/gagliardetto/solana-go v1.10.0


@nivasan1 do we have sufficient coverage to convince us that this is ok based on CI passing?

Eric-Warehime · 2024-04-08T19:22:31Z

pkg/math/testutils.go

+	if expected.Cmp(actual) > 0 {
+		diff = new(big.Float).Sub(new(big.Float).SetInt(expected), new(big.Float).SetInt(actual))
+	} else {
+		diff = new(big.Float).Sub(new(big.Float).SetInt(actual), new(big.Float).SetInt(expected))


Can't you just use the Abs function here?

I don't fully remember why i didn't use Abs but i do remember not using it for a specific reason. This code might change as we convert to big.Floats so I'd prefer leaving it as is for now.

Eric-Warehime · 2024-04-08T19:47:52Z

pkg/math/oracle/median_test.go

@@ -149,7 +150,7 @@ func TestAggregateData(t *testing.T) {
 			result := m.DataAggregator.GetAggregatedData()
 			require.Equal(t, len(tc.expectedPrices), len(result))
 			for ticker, price := range result {
-				verifyPrice(t, tc.expectedPrices[ticker], price)
+				math.VerifyPrice(t, tc.expectedPrices[ticker], price, acceptableDelta)


Why did these become non-deterministic now?

they aren't non-deterministic, they just put a bound on the amount of precision we expect in the resulting calculation. This only applies for big.Ints, for big.Floats we can directly specify precision so much of this might be deprecated with the changes that are following this.

Eric-Warehime · 2024-04-08T20:08:16Z

providers/apis/defi/uniswapv3/fetcher.go

+}
+
+// NewPriceFetcher returns a new Uniswap V3 price fetcher.
+func NewPriceFetcher(


nit: Untested.

davidterpay added 8 commits April 4, 2024 18:45

nits

f8ac0b0

nits

8d7caa6

nit

6e599c4

nitys

f7a352d

nit

cd68e02

testing start

83fe386

tidy

8533cc3

undo some stuff

10ae953

davidterpay force-pushed the terpay/uniswap-fetcher branch from e1a7bef to 10ae953 Compare April 4, 2024 22:46

more nits

4660d72

davidterpay added 2 commits April 4, 2024 19:32

testing nits

6045b9a

lint

7268b7e

davidterpay commented Apr 4, 2024

View reviewed changes

readme

21b77cf

davidterpay marked this pull request as ready for review April 4, 2024 23:50

davidterpay requested review from nivasan1, aljo242 and Eric-Warehime as code owners April 4, 2024 23:50

aljo242 reviewed Apr 4, 2024

View reviewed changes

pkg/math/math_test.go Outdated Show resolved Hide resolved

aljo242 reviewed Apr 4, 2024

View reviewed changes

pkg/math/math.go Outdated Show resolved Hide resolved

aljo242 reviewed Apr 4, 2024

View reviewed changes

pkg/math/test_utils.go Outdated Show resolved Hide resolved

aljo242 reviewed Apr 5, 2024

View reviewed changes

providers/apis/uniswapv3/fetcher.go Outdated Show resolved Hide resolved

providers/apis/uniswapv3/fetcher.go Outdated Show resolved Hide resolved

providers/apis/uniswapv3/fetcher_test.go Outdated Show resolved Hide resolved

providers/apis/uniswapv3/utils.go Outdated Show resolved Hide resolved

davidterpay added 3 commits April 4, 2024 20:13

cr1

815c294

Merge branch 'main' into terpay/uniswap-fetcher

a85020e

lint

357528a

davidterpay added 2 commits April 8, 2024 10:21

Merge branch 'main' into terpay/uniswap-fetcher

e409979

nits

720fcbb

aljo242 approved these changes Apr 8, 2024

View reviewed changes

davidterpay and others added 2 commits April 8, 2024 10:43

more nits

482a639

Merge branch 'main' into terpay/uniswap-fetcher

48d594c

nivasan1 reviewed Apr 8, 2024

View reviewed changes

Eric-Warehime reviewed Apr 8, 2024

View reviewed changes

Eric-Warehime approved these changes Apr 8, 2024

View reviewed changes

Merge branch 'main' into terpay/uniswap-fetcher

5c02f55

davidterpay merged commit 56fde74 into main Apr 9, 2024
14 of 15 checks passed

zrbecker deleted the terpay/uniswap-fetcher branch November 5, 2024 21:05

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat: UniswapV3 Provider #311

feat: UniswapV3 Provider #311

davidterpay commented Apr 4, 2024 •

edited

Loading

codecov bot commented Apr 4, 2024 •

edited

Loading

davidterpay Apr 4, 2024

aljo242 commented Apr 7, 2024

davidterpay commented Apr 8, 2024

nivasan1 Apr 8, 2024

nivasan1 left a comment

nivasan1 Apr 8, 2024

nivasan1 Apr 8, 2024

davidterpay Apr 8, 2024

Eric-Warehime Apr 8, 2024

Eric-Warehime Apr 8, 2024

Eric-Warehime Apr 8, 2024

Eric-Warehime Apr 8, 2024

davidterpay Apr 8, 2024

Eric-Warehime Apr 8, 2024

davidterpay Apr 8, 2024

Eric-Warehime Apr 8, 2024


		## Future Work

		The oracle side car is a combination of the oracle and provider packages. This is being moved to a [separate repository](https://github.com/skip-mev/slinky-sidecar).

feat: UniswapV3 Provider #311

feat: UniswapV3 Provider #311

Conversation

davidterpay commented Apr 4, 2024 • edited Loading

codecov bot commented Apr 4, 2024 • edited Loading

Codecov Report

Choose a reason for hiding this comment

aljo242 commented Apr 7, 2024

davidterpay commented Apr 8, 2024

Choose a reason for hiding this comment

nivasan1 left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

davidterpay commented Apr 4, 2024 •

edited

Loading

codecov bot commented Apr 4, 2024 •

edited

Loading