Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feature: introduce a ITokenFeeFetcher fallback #603

Closed
wants to merge 2 commits into from
Closed

feature: introduce a ITokenFeeFetcher fallback #603

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
4431cba
introduce a token fee fetcher fallback
xrsv Jun 12, 2024
608db22
Merge branch 'main' into xrsv/add-token-fee-fetcher-fallback
xrsv Jun 13, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
38 changes: 32 additions & 6 deletions src/providers/token-properties-provider.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -44,6 +44,7 @@ export class TokenPropertiesProvider implements ITokenPropertiesProvider {
private chainId: ChainId,
private tokenPropertiesCache: ICache<TokenPropertiesResult>,
private tokenFeeFetcher: ITokenFeeFetcher,
private tokenFeeFetcherFallback: ITokenFeeFetcher | undefined = undefined,
private allowList = DEFAULT_ALLOWLIST,
private positiveCacheEntryTTL = POSITIVE_CACHE_ENTRY_TTL,
private negativeCacheEntryTTL = NEGATIVE_CACHE_ENTRY_TTL
Expand All @@ -55,14 +56,15 @@ export class TokenPropertiesProvider implements ITokenPropertiesProvider {
): Promise<TokenPropertiesMap> {
const tokenToResult: TokenPropertiesMap = {};

// Note: Before enabling more ChainIds, make sure that provided tokenFeeFetchers supports them.
if (
!providerConfig?.enableFeeOnTransferFeeFetching ||
this.chainId !== ChainId.MAINNET
) {
return tokenToResult;
}

const addressesToFetchFeesOnchain: string[] = [];
const addressesToFetchFees: string[] = [];
const addressesRaw = this.buildAddressesRaw(tokens);
const addressesCacheKeys = this.buildAddressesCacheKeys(tokens);

Expand Down Expand Up @@ -103,27 +105,51 @@ export class TokenPropertiesProvider implements ITokenPropertiesProvider {
tokenValidationResult: TokenValidationResult.UNKN,
};
} else {
addressesToFetchFeesOnchain.push(address);
addressesToFetchFees.push(address);
}
}

if (addressesToFetchFeesOnchain.length > 0) {
if (addressesToFetchFees.length > 0) {
let tokenFeeMap: TokenFeeMap = {};

try {
tokenFeeMap = await this.tokenFeeFetcher.fetchFees(
addressesToFetchFeesOnchain,
addressesToFetchFees,
providerConfig
);
} catch (err) {
log.error(
{ err },
`Error fetching fees for tokens ${addressesToFetchFeesOnchain}`
`Error fetching fees for tokens ${addressesToFetchFees}`
);
}

if (this.tokenFeeFetcherFallback) {
// If a fallback fetcher is provided, we will use it to fetch fees for tokens that were not fetched by the primary fetcher.
const addressesToFetchFeesWithFallbackFetcher = addressesToFetchFees.filter(
(address) => !tokenFeeMap[address]
);
if (addressesToFetchFeesWithFallbackFetcher.length > 0) {
try {
const tokenFeeMapFromFallback = await this.tokenFeeFetcherFallback.fetchFees(
addressesToFetchFeesWithFallbackFetcher,
providerConfig
);
tokenFeeMap = {
...tokenFeeMap,
...tokenFeeMapFromFallback,
};
} catch (err) {
log.error(
{ err },
`Error fetching fees for tokens ${addressesToFetchFeesWithFallbackFetcher} using fallback`
);
}
}
}

Comment on lines +127 to +150
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What do you think about creating a GraphQlTokenFeeFetcherWithFallback implementation of ITokenFeeFetcher, and we put that in routing-api instead of here?

That way we could avoid all changes in SOR, we just need to make sure that the tokenFeeFetcher can be overwritten in routing-api.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

that's not a bad idea, let me do that.

Copy link
Member

@jsy1218 jsy1218 Jun 13, 2024 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I like @mikeki 's idea. Then in routing-api, we can create a traffic switch fetcher class (similar to lib/handlers/quote/provider-migration/v3/traffic-switch-on-chain-quote-provider.ts), that can achieve:

  1. beginning by shadow calling GQL token fetcher without affecting online path
  2. compare the results between GQL token fetcher and direct on-chain token fetcher
  3. gradually switching from direct on-chain token fetcher to GQL token fetcher
  4. and the existing traffic switch implementation already guarantees that in case GQL token fetcher throws error, it will fallback to use GQL token fetcher. We will just have to tweak the implementation a bit, so that when GQL token fetcher returns 0 fee, it also fallback to direct on-chain token fetcher to try fetching the FOT fee again. Since direct on-chain token fetcher is wrapped with in-memory cache, perf penalty in those fallbacks are minimized.

await Promise.all(
addressesToFetchFeesOnchain.map((address) => {
addressesToFetchFees.map((address) => {
const tokenFee = tokenFeeMap[address];
const tokenFeeResultExists: BigNumber | undefined =
tokenFee && (tokenFee.buyFeeBps || tokenFee.sellFeeBps);
Expand Down
162 changes: 162 additions & 0 deletions test/unit/providers/token-properties-provider.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -164,6 +164,168 @@ describe('TokenPropertiesProvider', () => {
}
});


it('succeeds to get token properties in a single batch when main fetcher fails but fallback succeeds', async function() {

let mockTokenFeeFetcherFallback: sinon.SinonStubbedInstance<ITokenFeeFetcher>
mockTokenFeeFetcherFallback = sinon.createStubInstance(OnChainTokenFeeFetcher)

const underlyingCache: NodeCache = new NodeCache({ stdTTL: 3600, useClones: false })
const tokenPropertiesResultCache: NodeJSCache<TokenPropertiesResult> = new NodeJSCache(underlyingCache);
const tokenPropertiesProvider = new TokenPropertiesProvider(
ChainId.MAINNET,
tokenPropertiesResultCache,
mockTokenFeeFetcher,
mockTokenFeeFetcherFallback
)
const currentEpochTimeInSeconds = Math.floor(Date.now() / 1000);

const token1 = new Token(1, '0x0000000000000000000000000000000000000012', 18);
const token2 = new Token(1, '0x0000000000000000000000000000000000000034', 18);
const token3 = new Token(1, '0x0000000000000000000000000000000000000056', 18);

const tokens = [token1, token2, token3]

mockTokenFeeFetcher.fetchFees.callsFake(async (_) => {
throw new Error("Something went wrong")
});

mockTokenFeeFetcherFallback.fetchFees.callsFake(async (addresses) => {
const tokenToResult: TokenFeeMap = {};
addresses.forEach((address) => {
tokenToResult[address] = {
buyFeeBps: BigNumber.from(parseInt(address[address.length - 2]!)),
sellFeeBps: BigNumber.from(parseInt(address[address.length - 1]!))
}
});

return tokenToResult
});

const tokenPropertiesMap = await tokenPropertiesProvider.getTokensProperties(tokens, { enableFeeOnTransferFeeFetching: true });

for (const token of tokens) {
const address = token.address.toLowerCase()
expect(tokenPropertiesMap[address]).toBeDefined();
expect(tokenPropertiesMap[address]?.tokenFeeResult).toBeDefined();
const expectedBuyFeeBps = tokenPropertiesMap[address]?.tokenFeeResult?.buyFeeBps
const expectedSellFeeBps = tokenPropertiesMap[address]?.tokenFeeResult?.sellFeeBps
assertExpectedTokenProperties(tokenPropertiesMap[address], expectedBuyFeeBps, expectedSellFeeBps, TokenValidationResult.FOT);

const cachedTokenProperties = await tokenPropertiesResultCache.get(CACHE_KEY(ChainId.MAINNET, token.address.toLowerCase()))
expect(cachedTokenProperties).toBeDefined();
assertExpectedTokenProperties(cachedTokenProperties, expectedBuyFeeBps, expectedSellFeeBps, TokenValidationResult.FOT);

underlyingCache.getTtl(CACHE_KEY(ChainId.MAINNET, token.address.toLowerCase()))
expect(Math.floor((underlyingCache.getTtl(CACHE_KEY(ChainId.MAINNET, token.address.toLowerCase())) ?? 0) / 1000)).toEqual(currentEpochTimeInSeconds + POSITIVE_CACHE_ENTRY_TTL);
}
});


it('succeeds to get token properties, some from primary fetcher, some from secondary fetcher', async function() {

let mockTokenFeeFetcherFallback: sinon.SinonStubbedInstance<ITokenFeeFetcher>
mockTokenFeeFetcherFallback = sinon.createStubInstance(OnChainTokenFeeFetcher)

const underlyingCache: NodeCache = new NodeCache({ stdTTL: 3600, useClones: false })
const tokenPropertiesResultCache: NodeJSCache<TokenPropertiesResult> = new NodeJSCache(underlyingCache);
const tokenPropertiesProvider = new TokenPropertiesProvider(
ChainId.MAINNET,
tokenPropertiesResultCache,
mockTokenFeeFetcher,
mockTokenFeeFetcherFallback
)
const currentEpochTimeInSeconds = Math.floor(Date.now() / 1000);

const token1 = new Token(1, '0x0000000000000000000000000000000000000012', 18);
const token2 = new Token(1, '0x0000000000000000000000000000000000000034', 18);
const token3 = new Token(1, '0x0000000000000000000000000000000000000056', 18);

const tokens = [token1, token2, token3]

mockTokenFeeFetcher.fetchFees.callsFake(async (_) => {
return {
'0x0000000000000000000000000000000000000012': {
'buyFeeBps': BigNumber.from(213),
'sellFeeBps': BigNumber.from(800)
},
'0x0000000000000000000000000000000000000034': {
'buyFeeBps': BigNumber.from(213),
'sellFeeBps': BigNumber.from(800)
}
}
});

mockTokenFeeFetcherFallback.fetchFees.callsFake(async (_) => {
return {
'0x0000000000000000000000000000000000000056': {
'buyFeeBps': BigNumber.from(213),
'sellFeeBps': BigNumber.from(800)
}
}
});

const tokenPropertiesMap = await tokenPropertiesProvider.getTokensProperties(tokens, { enableFeeOnTransferFeeFetching: true });

for (const token of tokens) {
const address = token.address.toLowerCase()
expect(tokenPropertiesMap[address]).toBeDefined();
expect(tokenPropertiesMap[address]?.tokenFeeResult).toBeDefined();
const expectedBuyFeeBps = tokenPropertiesMap[address]?.tokenFeeResult?.buyFeeBps
const expectedSellFeeBps = tokenPropertiesMap[address]?.tokenFeeResult?.sellFeeBps
assertExpectedTokenProperties(tokenPropertiesMap[address], expectedBuyFeeBps, expectedSellFeeBps, TokenValidationResult.FOT);

const cachedTokenProperties = await tokenPropertiesResultCache.get(CACHE_KEY(ChainId.MAINNET, token.address.toLowerCase()))
expect(cachedTokenProperties).toBeDefined();
assertExpectedTokenProperties(cachedTokenProperties, expectedBuyFeeBps, expectedSellFeeBps, TokenValidationResult.FOT);

underlyingCache.getTtl(CACHE_KEY(ChainId.MAINNET, token.address.toLowerCase()))
expect(Math.floor((underlyingCache.getTtl(CACHE_KEY(ChainId.MAINNET, token.address.toLowerCase())) ?? 0) / 1000)).toEqual(currentEpochTimeInSeconds + POSITIVE_CACHE_ENTRY_TTL);
}
});


it('all token fee fetch failed for primary and fallback fetcher', async function() {
let mockTokenFeeFetcherFallback: sinon.SinonStubbedInstance<ITokenFeeFetcher>
mockTokenFeeFetcherFallback = sinon.createStubInstance(OnChainTokenFeeFetcher)

const underlyingCache: NodeCache = new NodeCache({ stdTTL: 3600, useClones: false })
const tokenPropertiesResultCache: NodeJSCache<TokenPropertiesResult> = new NodeJSCache(underlyingCache);
const tokenPropertiesProvider = new TokenPropertiesProvider(
ChainId.MAINNET,
tokenPropertiesResultCache,
mockTokenFeeFetcher,
mockTokenFeeFetcherFallback
)
const currentEpochTimeInSeconds = Math.floor(Date.now() / 1000);

const token1 = new Token(1, '0x0000000000000000000000000000000000000012', 18);
const token2 = new Token(1, '0x0000000000000000000000000000000000000034', 18);
const token3 = new Token(1, '0x0000000000000000000000000000000000000056', 18);

const tokens = [token1, token2, token3]

mockTokenFeeFetcher.fetchFees.withArgs(tokens.map(token => token.address)).throws(new Error('Failed to fetch fees for token 1'));
mockTokenFeeFetcherFallback.fetchFees.withArgs(tokens.map(token => token.address)).throws(new Error('Failed to fetch fees for token 1'));

const tokenPropertiesMap = await tokenPropertiesProvider.getTokensProperties(tokens, { enableFeeOnTransferFeeFetching: true });

for (const token of tokens) {
const address = token.address.toLowerCase()
expect(tokenPropertiesMap[address]).toBeDefined();
expect(tokenPropertiesMap[address]?.tokenFeeResult).toBeUndefined();
expect(tokenPropertiesMap[address]?.tokenValidationResult).toBeUndefined();

const cachedTokenProperties = await tokenPropertiesResultCache.get(CACHE_KEY(ChainId.MAINNET, token.address.toLowerCase()))
expect(cachedTokenProperties).toBeDefined();
expect(cachedTokenProperties?.tokenFeeResult).toBeUndefined();
expect(cachedTokenProperties?.tokenValidationResult).toBeUndefined();

underlyingCache.getTtl(CACHE_KEY(ChainId.MAINNET, token.address.toLowerCase()))
expect(Math.floor((underlyingCache.getTtl(CACHE_KEY(ChainId.MAINNET, token.address.toLowerCase())) ?? 0) / 1000)).toEqual(currentEpochTimeInSeconds + NEGATIVE_CACHE_ENTRY_TTL);
}
});

it('all token fee fetch failed', async function() {
const underlyingCache: NodeCache = new NodeCache({ stdTTL: 3600, useClones: false })
const tokenPropertiesResultCache: NodeJSCache<TokenPropertiesResult> = new NodeJSCache(underlyingCache);
Expand Down
Loading