[feat] [ROUTE-9] v3 dynamo cache pool provider #201
Conversation
| import { FeeAmount, Pool } from '@uniswap/v3-sdk' | ||
| import { DynamoDB } from 'aws-sdk' | ||
|
|
||
| export class DynamoPoolProvider implements IV3PoolProvider { |
There was a problem hiding this comment.
@mikeki I find it a bit awkward that I'm copying and pasting a lot of code from caching-pool-provider.ts, but I think it's better to minimize the refactoring in smart-order-router right? Also in this case, we will be able to delete caching-pool-provider.ts upon migration to dynamo cache.
There was a problem hiding this comment.
I think you no longer are doing alot of copy pasting?
There was a problem hiding this comment.
With the introduction of cache-dynamo.ts, I see an opportunity to avoid a lot of copy paste from caching-pool-provider.ts, as well as subsequent refactoring with dynamo-route-caching-provider.ts.
On high-level I think the dynamo caching can be abstracted into a common interface, which is my first attempt in this PR. However, I don't want to over expand the scope and keep the refactoring in future chance.
jsy1218
changed the title
lib/handlers/router-entities/pool-caching/v3/dynamo-pool-provider.ts
Outdated
Show resolved
Hide resolved
| import { FeeAmount, Pool } from '@uniswap/v3-sdk' | ||
| import { DynamoDB } from 'aws-sdk' | ||
|
|
||
| export class DynamoPoolProvider implements IV3PoolProvider { |
There was a problem hiding this comment.
I think you no longer are doing alot of copy pasting?
lib/handlers/router-entities/pool-caching/v3/dynamo-pool-provider.ts
Outdated
Show resolved
Hide resolved
lib/handlers/router-entities/pool-caching/v3/dynamo-pool-provider.ts
Outdated
Show resolved
Hide resolved
jsy1218
changed the title
jsy1218
changed the title
lib/handlers/router-entities/pool-caching/v3/cache-dynamo-pool.ts
Outdated
Show resolved
Hide resolved
| TableName: this.tableName, | ||
| Key: { | ||
| poolAddress: partitionKey, | ||
| blockNumber: sortKey, |
There was a problem hiding this comment.
should we add some way of fetching more than just the last block? I think eventually (specially with V3 Pool Provider) we should be able to look at some past blocks to speed things up, wdyt?
There was a problem hiding this comment.
aw what's the use case for fetching some past blocks? Is it when we don't know what's the latest the block number?
There was a problem hiding this comment.
In case we want to optimistically use the previous block data, but I think we can go with what we have right now, and then see how it performs before making other changes :)
lib/handlers/router-entities/pool-caching/v3/cache-dynamo-pool.ts
Outdated
Show resolved
Hide resolved
jsy1218
deleted the
jsy1218/replace-in-memory-cache-with-dynamo-cache-for-pool
branch
What?
In routing-api, we create a CachingV3PoolProvider which is defined in SOR.
We can create a new DynamoDBCachingV3PoolProvider in the routing-api source code that implements IV3PoolProvider, and uses a DynamoDB Table for storing the Pools into cache.
It is important to use the block number as a sort key for the cache to make sure that we are storing the information of the pools at a given block height, and for consistency of quotes.
Why?
We see misquotes across v2 and v3. We think the existing in-memory cache is one source of the issue.
How?
We will begin replacing v3 in-memory cache with distributed cache.
Testing?
All unit tests and integ tests passed.
Screenshots (optional)
N/A
Anything Else?
BlockNumber are passed into V2Quoter and MixedQuoter, but not V3Quoter . I think we would want to remain as is for now, and test the MixedQuoter after dynamo caching V3.