Refactor PoolsCache (part 1) [TKR-500] (#525)

* Make _refreshPoolCacheIfRequiredAsync type-safe and remove Promise.all

* Factor out PoolsCache key logic into a function

* Use Map instead of object in PoolsCache and increase the default timeout

* Clean up PoolsCache and simplify its public interface
This commit is contained in:
Kyu 2022-07-28 09:04:42 -07:00 committed by GitHub
parent 9856e78609
commit 14dcee5bb6
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
8 changed files with 93 additions and 109 deletions

View File

@ -862,14 +862,9 @@ export class MarketOperationUtils {
} }
private async _refreshPoolCacheIfRequiredAsync(takerToken: string, makerToken: string): Promise<void> { private async _refreshPoolCacheIfRequiredAsync(takerToken: string, makerToken: string): Promise<void> {
void Promise.all( _.values(this._sampler.poolsCaches)
Object.values(this._sampler.poolsCaches).map(async cache => { .filter(cache => cache !== undefined && !cache.isFresh(takerToken, makerToken))
if (!cache || cache.isFresh(takerToken, makerToken)) { .forEach(cache => cache?.getFreshPoolsForPairAsync(takerToken, makerToken));
return Promise.resolve([]);
}
return cache.getFreshPoolsForPairAsync(takerToken, makerToken);
}),
);
} }
} }

View File

@ -23,7 +23,7 @@ interface BalancerPoolResponse {
export class BalancerPoolsCache extends PoolsCache { export class BalancerPoolsCache extends PoolsCache {
constructor( constructor(
private readonly _subgraphUrl: string = BALANCER_SUBGRAPH_URL, private readonly _subgraphUrl: string = BALANCER_SUBGRAPH_URL,
cache: { [key: string]: CacheValue } = {}, cache: Map<string, CacheValue> = new Map(),
private readonly maxPoolsFetched: number = BALANCER_MAX_POOLS_FETCHED, private readonly maxPoolsFetched: number = BALANCER_MAX_POOLS_FETCHED,
private readonly _topPoolsFetched: number = BALANCER_TOP_POOLS_FETCHED, private readonly _topPoolsFetched: number = BALANCER_TOP_POOLS_FETCHED,
private readonly _warningLogger: LogFunction = DEFAULT_WARNING_LOGGER, private readonly _warningLogger: LogFunction = DEFAULT_WARNING_LOGGER,

View File

@ -63,7 +63,7 @@ export class BalancerV2PoolsCache extends PoolsCache {
private readonly maxPoolsFetched: number = BALANCER_MAX_POOLS_FETCHED, private readonly maxPoolsFetched: number = BALANCER_MAX_POOLS_FETCHED,
private readonly _topPoolsFetched: number = BALANCER_TOP_POOLS_FETCHED, private readonly _topPoolsFetched: number = BALANCER_TOP_POOLS_FETCHED,
private readonly _warningLogger: LogFunction = DEFAULT_WARNING_LOGGER, private readonly _warningLogger: LogFunction = DEFAULT_WARNING_LOGGER,
cache: { [key: string]: CacheValue } = {}, cache: Map<string, CacheValue> = new Map(),
) { ) {
super(cache); super(cache);
void this._loadTopPoolsAsync(); void this._loadTopPoolsAsync();

View File

@ -7,7 +7,7 @@ import { CacheValue, PoolsCache } from './pools_cache';
export class CreamPoolsCache extends PoolsCache { export class CreamPoolsCache extends PoolsCache {
constructor( constructor(
_cache: { [key: string]: CacheValue } = {}, _cache: Map<string, CacheValue> = new Map(),
private readonly maxPoolsFetched: number = BALANCER_MAX_POOLS_FETCHED, private readonly maxPoolsFetched: number = BALANCER_MAX_POOLS_FETCHED,
) { ) {
super(_cache); super(_cache);

View File

@ -10,15 +10,23 @@ export interface CacheValue {
// tslint:disable:custom-no-magic-numbers // tslint:disable:custom-no-magic-numbers
// Cache results for 30mins // Cache results for 30mins
const DEFAULT_CACHE_TIME_MS = (ONE_HOUR_IN_SECONDS / 2) * ONE_SECOND_MS; const DEFAULT_CACHE_TIME_MS = (ONE_HOUR_IN_SECONDS / 2) * ONE_SECOND_MS;
const DEFAULT_TIMEOUT_MS = 1000; const DEFAULT_TIMEOUT_MS = 3000;
// tslint:enable:custom-no-magic-numbers // tslint:enable:custom-no-magic-numbers
export abstract class PoolsCache { export abstract class PoolsCache {
protected static _isExpired(value: CacheValue): boolean { protected static _getKey(takerToken: string, makerToken: string): string {
return `${takerToken}-${makerToken}`;
}
protected static _isExpired(value: CacheValue | undefined): boolean {
if (value === undefined) {
return true;
}
return Date.now() >= value.expiresAt; return Date.now() >= value.expiresAt;
} }
constructor( constructor(
protected readonly _cache: { [key: string]: CacheValue }, protected readonly _cache: Map<string, CacheValue>,
protected readonly _cacheTimeMs: number = DEFAULT_CACHE_TIME_MS, protected readonly _cacheTimeMs: number = DEFAULT_CACHE_TIME_MS,
) {} ) {}
@ -31,47 +39,42 @@ export abstract class PoolsCache {
return Promise.race([this._getAndSaveFreshPoolsForPairAsync(takerToken, makerToken), timeout]); return Promise.race([this._getAndSaveFreshPoolsForPairAsync(takerToken, makerToken), timeout]);
} }
public getCachedPoolAddressesForPair( /**
takerToken: string, * Returns pool addresses (can be stale) for a pair.
makerToken: string, *
ignoreExpired: boolean = true, * An empty array will be returned if cache does not exist.
): string[] | undefined { */
const key = JSON.stringify([takerToken, makerToken]); public getPoolAddressesForPair(takerToken: string, makerToken: string): string[] {
const value = this._cache[key]; const value = this._getValue(takerToken, makerToken);
if (ignoreExpired) {
return value === undefined ? [] : value.pools.map(pool => pool.id); return value === undefined ? [] : value.pools.map(pool => pool.id);
} }
if (!value) {
return undefined;
}
if (PoolsCache._isExpired(value)) {
return undefined;
}
return (value || []).pools.map(pool => pool.id);
}
public isFresh(takerToken: string, makerToken: string): boolean { public isFresh(takerToken: string, makerToken: string): boolean {
const cached = this.getCachedPoolAddressesForPair(takerToken, makerToken, false); const value = this._getValue(takerToken, makerToken);
return cached !== undefined; return !PoolsCache._isExpired(value);
}
protected _getValue(takerToken: string, makerToken: string): CacheValue | undefined {
const key = PoolsCache._getKey(takerToken, makerToken);
return this._cache.get(key);
} }
protected async _getAndSaveFreshPoolsForPairAsync(takerToken: string, makerToken: string): Promise<Pool[]> { protected async _getAndSaveFreshPoolsForPairAsync(takerToken: string, makerToken: string): Promise<Pool[]> {
const key = JSON.stringify([takerToken, makerToken]); const key = PoolsCache._getKey(takerToken, makerToken);
const value = this._cache[key]; const value = this._cache.get(key);
if (value === undefined || value.expiresAt >= Date.now()) { if (!PoolsCache._isExpired(value)) {
return value!.pools;
}
const pools = await this._fetchPoolsForPairAsync(takerToken, makerToken); const pools = await this._fetchPoolsForPairAsync(takerToken, makerToken);
const expiresAt = Date.now() + this._cacheTimeMs; const expiresAt = Date.now() + this._cacheTimeMs;
this._cachePoolsForPair(takerToken, makerToken, pools, expiresAt); this._cachePoolsForPair(takerToken, makerToken, pools, expiresAt);
} return pools;
return this._cache[key].pools;
} }
protected _cachePoolsForPair(takerToken: string, makerToken: string, pools: Pool[], expiresAt: number): void { protected _cachePoolsForPair(takerToken: string, makerToken: string, pools: Pool[], expiresAt: number): void {
const key = JSON.stringify([takerToken, makerToken]); const key = PoolsCache._getKey(takerToken, makerToken);
this._cache[key] = { this._cache.set(key, { pools, expiresAt });
pools,
expiresAt,
};
} }
protected abstract _fetchPoolsForPairAsync(takerToken: string, makerToken: string): Promise<Pool[]>; protected abstract _fetchPoolsForPairAsync(takerToken: string, makerToken: string): Promise<Pool[]>;

View File

@ -1592,12 +1592,9 @@ export class SamplerOperations {
), ),
]; ];
case ERC20BridgeSource.Balancer: case ERC20BridgeSource.Balancer:
return ( return this.poolsCaches[ERC20BridgeSource.Balancer]
this.poolsCaches[ERC20BridgeSource.Balancer].getCachedPoolAddressesForPair( .getPoolAddressesForPair(takerToken, makerToken)
takerToken, .map(balancerPool =>
makerToken,
) || []
).map(balancerPool =>
this.getBalancerSellQuotes( this.getBalancerSellQuotes(
balancerPool, balancerPool,
makerToken, makerToken,
@ -1627,15 +1624,14 @@ export class SamplerOperations {
if (cache === undefined) { if (cache === undefined) {
return []; return [];
} }
const poolIds = cache.getCachedPoolAddressesForPair(takerToken, makerToken) || []; const poolAddresses = cache.getPoolAddressesForPair(takerToken, makerToken);
const vault = BEETHOVEN_X_VAULT_ADDRESS_BY_CHAIN[this.chainId]; const vault = BEETHOVEN_X_VAULT_ADDRESS_BY_CHAIN[this.chainId];
if (vault === NULL_ADDRESS) { if (vault === NULL_ADDRESS) {
return []; return [];
} }
return poolIds.map(poolId => return poolAddresses.map(poolAddress =>
this.getBalancerV2SellQuotes( this.getBalancerV2SellQuotes(
{ poolId, vault }, { poolId: poolAddress, vault },
makerToken, makerToken,
takerToken, takerToken,
takerFillAmounts, takerFillAmounts,
@ -1644,12 +1640,9 @@ export class SamplerOperations {
); );
} }
case ERC20BridgeSource.Cream: case ERC20BridgeSource.Cream:
return ( return this.poolsCaches[ERC20BridgeSource.Cream]
this.poolsCaches[ERC20BridgeSource.Cream].getCachedPoolAddressesForPair( .getPoolAddressesForPair(takerToken, makerToken)
takerToken, .map(creamPool =>
makerToken,
) || []
).map(creamPool =>
this.getBalancerSellQuotes( this.getBalancerSellQuotes(
creamPool, creamPool,
makerToken, makerToken,
@ -1948,12 +1941,9 @@ export class SamplerOperations {
), ),
]; ];
case ERC20BridgeSource.Balancer: case ERC20BridgeSource.Balancer:
return ( return this.poolsCaches[ERC20BridgeSource.Balancer]
this.poolsCaches[ERC20BridgeSource.Balancer].getCachedPoolAddressesForPair( .getPoolAddressesForPair(takerToken, makerToken)
takerToken, .map(poolAddress =>
makerToken,
) || []
).map(poolAddress =>
this.getBalancerBuyQuotes( this.getBalancerBuyQuotes(
poolAddress, poolAddress,
makerToken, makerToken,
@ -1989,8 +1979,7 @@ export class SamplerOperations {
if (cache === undefined) { if (cache === undefined) {
return []; return [];
} }
const poolIds = cache.getCachedPoolAddressesForPair(takerToken, makerToken) || []; const poolIds = cache.getPoolAddressesForPair(takerToken, makerToken) || [];
const vault = BEETHOVEN_X_VAULT_ADDRESS_BY_CHAIN[this.chainId]; const vault = BEETHOVEN_X_VAULT_ADDRESS_BY_CHAIN[this.chainId];
if (vault === NULL_ADDRESS) { if (vault === NULL_ADDRESS) {
return []; return [];
@ -2006,12 +1995,9 @@ export class SamplerOperations {
); );
} }
case ERC20BridgeSource.Cream: case ERC20BridgeSource.Cream:
return ( return this.poolsCaches[ERC20BridgeSource.Cream]
this.poolsCaches[ERC20BridgeSource.Cream].getCachedPoolAddressesForPair( .getPoolAddressesForPair(takerToken, makerToken)
takerToken, .map(poolAddress =>
makerToken,
) || []
).map(poolAddress =>
this.getBalancerBuyQuotes( this.getBalancerBuyQuotes(
poolAddress, poolAddress,
makerToken, makerToken,

View File

@ -100,7 +100,7 @@ async function getMarketBuyOrdersAsync(
class MockPoolsCache extends PoolsCache { class MockPoolsCache extends PoolsCache {
constructor(private readonly _handler: (takerToken: string, makerToken: string) => Pool[]) { constructor(private readonly _handler: (takerToken: string, makerToken: string) => Pool[]) {
super({}); super(new Map());
} }
protected async _fetchPoolsForPairAsync(takerToken: string, makerToken: string): Promise<Pool[]> { protected async _fetchPoolsForPairAsync(takerToken: string, makerToken: string): Promise<Pool[]> {
return this._handler(takerToken, makerToken); return this._handler(takerToken, makerToken);

View File

@ -28,7 +28,7 @@ describe('Pools Caches for Balancer-based sampling', () => {
expect(pools.length).greaterThan(0, `Failed to find any pools for ${takerToken} and ${makerToken}`); expect(pools.length).greaterThan(0, `Failed to find any pools for ${takerToken} and ${makerToken}`);
expect(pools[0]).not.undefined(); expect(pools[0]).not.undefined();
expect(Object.keys(pools[0])).to.include.members(poolKeys); expect(Object.keys(pools[0])).to.include.members(poolKeys);
const cachedPoolIds = cache.getCachedPoolAddressesForPair(takerToken, makerToken); const cachedPoolIds = cache.getPoolAddressesForPair(takerToken, makerToken);
expect(cachedPoolIds).to.deep.equal(pools.map(p => p.id)); expect(cachedPoolIds).to.deep.equal(pools.map(p => p.id));
} }