-
Notifications
You must be signed in to change notification settings - Fork 16
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.
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
feat(permit): with token name #312
Changes from 21 commits
e00399f
cede9da
079b90f
1336fa4
18c8268
cf728ac
64a72d2
ee190d9
4dade58
2f8d2ce
cb0a127
ef56db8
21e345c
505aa38
dcd9dc7
814c5fd
015b755
c63455e
661c9cd
8107d7b
868f4e2
ccf93e3
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -24,9 +24,17 @@ | |
* @arg chainId - required, first positional argument | ||
* @arg tokenListPath - optional, second positional argument | ||
* @arg rpcUrl - optional, third positional argument | ||
* @arg recheckUnsupported - optional, fourth positional argument | ||
*/ | ||
|
||
import { getTokenPermitInfo, PermitInfo } from '@cowprotocol/permit-utils' | ||
import pThrottle from 'p-throttle' | ||
import pRetry from 'p-retry' | ||
import { | ||
getTokenPermitInfo, | ||
GetTokenPermitIntoResult, | ||
isSupportedPermitInfo, | ||
PermitInfo, | ||
} from '@cowprotocol/permit-utils' | ||
import * as path from 'node:path' | ||
import { readFileSync, writeFileSync } from 'node:fs' | ||
import { JsonRpcProvider } from '@ethersproject/providers' | ||
|
@@ -35,11 +43,12 @@ import { BASE_PATH, SPENDER_ADDRESS } from './const.ts' | |
import { sortPermitInfo } from './utils/sortPermitInfo.ts' | ||
import { getProvider } from './utils/getProvider.ts' | ||
import { Token } from './types.ts' | ||
import { getTokens } from './utils/getTokens.ts' | ||
import { getTokensFromTokenList } from './utils/getTokensFromTokenList.ts' | ||
import { getUnsupportedTokensFromPermitInfo } from './utils/getUnsupportedTokensFromPermitInfo.ts' | ||
|
||
// TODO: maybe make the args nicer? | ||
// Get args from cli: chainId, optional token lists path, optional rpcUrl | ||
const [, scriptPath, chainId, tokenListPath, rpcUrl] = argv | ||
// Get args from cli: chainId, optional token lists path, optional rpcUrl, optional recheckUnsupported flag | ||
const [, scriptPath, chainId, tokenListPath, rpcUrl, recheckUnsupported] = argv | ||
|
||
if (!chainId) { | ||
console.error('ChainId is missing. Invoke the script with the chainId as the first parameter.') | ||
|
@@ -49,10 +58,12 @@ if (!chainId) { | |
// Change to script dir so relative paths work properly | ||
chdir(path.dirname(scriptPath)) | ||
|
||
|
||
async function fetchPermitInfo( | ||
chainId: number, | ||
tokenListPath: string | undefined, | ||
rpcUrl: string | undefined, | ||
recheckUnsupported: boolean = false, | ||
): Promise<void> { | ||
// Load existing permitInfo.json file for given chainId | ||
const permitInfoPath = path.join(BASE_PATH, `PermitInfo.${chainId}.json`) | ||
|
@@ -64,32 +75,41 @@ async function fetchPermitInfo( | |
allPermitInfo = JSON.parse(readFileSync(permitInfoPath, 'utf8')) as Record<string, PermitInfo> | ||
} catch (_) { | ||
// File doesn't exist. It'll be created later on. | ||
if (recheckUnsupported) { | ||
console.error('recheck option set without existing permitInfo. There is nothing to recheck') | ||
exit(1) | ||
} | ||
} | ||
|
||
// Build provider instance | ||
const provider = getProvider(chainId, rpcUrl) | ||
|
||
// Load tokens info from a token list | ||
const tokens = getTokens(chainId, tokenListPath) | ||
const tokens = recheckUnsupported | ||
? getUnsupportedTokensFromPermitInfo(chainId, allPermitInfo) | ||
: getTokensFromTokenList(chainId, tokenListPath) | ||
|
||
// Create a list of promises to check all tokens | ||
const fetchAllPermits = tokens.map((token) => { | ||
const existingInfo = allPermitInfo[token.address.toLowerCase()] | ||
|
||
return _fetchPermitInfo(chainId, provider, token, existingInfo) | ||
return pRetry(async () => _fetchPermitInfo(chainId, provider, token, existingInfo, recheckUnsupported), { | ||
retries: 3, | ||
}) | ||
}) | ||
|
||
// Await for all of them to complete | ||
const fetchedPermits = await Promise.allSettled(fetchAllPermits) | ||
|
||
// Iterate over each result | ||
fetchedPermits.forEach((result) => { | ||
// Ignore failed or the ones where the value is falsy | ||
if (result.status === 'fulfilled' && result.value) { | ||
const [address, permitInfo] = result.value | ||
|
||
// Store result | ||
allPermitInfo[address] = permitInfo | ||
} else if (result.status === 'rejected') { | ||
console.log(`[fetchedPermits] Failed to fetch info:`, result.reason) | ||
} | ||
}) | ||
|
||
|
@@ -100,37 +120,49 @@ async function fetchPermitInfo( | |
} | ||
} | ||
|
||
// Fn can only be called 2x/second | ||
const throttle = pThrottle({ | ||
limit: 2, | ||
interval: 1000, | ||
}) | ||
|
||
const throttledGetTokenPermitInfo = throttle(getTokenPermitInfo) | ||
Comment on lines
+123
to
+129
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Throttling the call to try and avoid RPC failures. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Does it fail a lot? I mean, how many calls w e do per token? shouldn't break if we use our RPC (nodereal) There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Made the script more robust, and added retries. |
||
|
||
async function _fetchPermitInfo( | ||
chainId: number, | ||
provider: JsonRpcProvider, | ||
token: Token, | ||
existing: PermitInfo | undefined, | ||
recheckUnsupported: boolean, | ||
): Promise<undefined | [string, PermitInfo]> { | ||
if (existing !== undefined) { | ||
console.info(`Token ${token.symbol}: already known, skipping`, existing) | ||
} else if (token.chainId !== chainId) { | ||
console.info(`Token ${token.symbol}: belongs to a different network (${token.chainId}), skipping`) | ||
const tokenId = token.symbol || token.name || token.address | ||
|
||
if (token.chainId !== chainId) { | ||
console.info(`Token ${tokenId}: belongs to a different network (${token.chainId}), skipping`) | ||
} else if (isSupportedPermitInfo(existing) || (existing && !recheckUnsupported)) { | ||
console.info(`Token ${tokenId}: already known, skipping`, existing) | ||
} else { | ||
try { | ||
const response = await getTokenPermitInfo({ | ||
chainId, | ||
provider, | ||
spender: SPENDER_ADDRESS, | ||
tokenAddress: token.address, | ||
tokenName: token.name, | ||
}) | ||
console.info(`Token ${token.symbol}:`, response) | ||
|
||
// Ignore error responses | ||
if (!(typeof response === 'object' && 'error' in response)) { | ||
return [token.address.toLowerCase(), response] | ||
const response: GetTokenPermitIntoResult = await throttledGetTokenPermitInfo({ | ||
chainId, | ||
provider, | ||
spender: SPENDER_ADDRESS, | ||
tokenAddress: token.address, | ||
tokenName: token.name, | ||
}) | ||
|
||
if ('error' in response) { | ||
if (/ETIMEDOUT|RPC connection error/.test(response.error)) { | ||
// Throw, so it can be retried on connection errors | ||
throw new Error(response.error) | ||
} | ||
} catch (e) { | ||
// Ignore failures | ||
console.info(`Failed ${token.symbol}:`, e) | ||
// Non connection related error, stop it here | ||
console.info(`Non-retryable failure for token ${tokenId}:`, response) | ||
} else { | ||
console.info(`Token ${tokenId}:`, response) | ||
return [token.address.toLowerCase(), response] | ||
} | ||
} | ||
} | ||
|
||
// Execute the script | ||
fetchPermitInfo(+chainId, tokenListPath, rpcUrl).then(() => console.info(`Done 🏁`)) | ||
fetchPermitInfo(+chainId, tokenListPath, rpcUrl, !!recheckUnsupported).then(() => console.info(`Done 🏁`)) |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -6,35 +6,32 @@ | |
"type": "object", | ||
"patternProperties": { | ||
"^0x[a-fA-F0-9]{40}$": { | ||
"oneOf": [ | ||
{ | ||
"type": "object", | ||
"title": "PermitInfo", | ||
"description": "Individual permit info when a token is known to be permittable", | ||
"properties": { | ||
"version": { | ||
"type": "string", | ||
"description": "Optional version number, as a string", | ||
"pattern": "^\\d+$" | ||
}, | ||
"type": { | ||
"type": "string", | ||
"description": "Type of permit", | ||
"enum": [ | ||
"eip-2612", | ||
"dai-like" | ||
] | ||
} | ||
}, | ||
"required": [ | ||
"type" | ||
"type": "object", | ||
"title": "PermitInfo", | ||
"description": "Individual permit info when a token is known to be permittable", | ||
"properties": { | ||
"version": { | ||
"type": "string", | ||
"description": "Optional version, natural number > 0, as a string", | ||
"pattern": "^\\d+$" | ||
}, | ||
"type": { | ||
"type": "string", | ||
"description": "Type of permit", | ||
"enum": [ | ||
"unsupported", | ||
"eip-2612", | ||
"dai-like" | ||
] | ||
}, | ||
{ | ||
"type": "boolean", | ||
"description": "When a token is known to not be permittable", | ||
"const": false | ||
Comment on lines
-33
to
-36
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. No longer have the type with only |
||
"name": { | ||
"type": "string", | ||
"description": "Token name as defined in the contract" | ||
Comment on lines
+27
to
+29
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Added |
||
} | ||
}, | ||
"required": [ | ||
"type", | ||
"name" | ||
] | ||
} | ||
}, | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,6 +1,6 @@ | ||
export type Token = { | ||
address: string | ||
name: string | ||
chainId: number | ||
symbol: string | ||
name?: string | ||
symbol?: string | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -3,7 +3,7 @@ import { BASE_PATH } from '../const.ts' | |
import { Token } from '../types.ts' | ||
import { join } from 'node:path' | ||
|
||
export function getTokens(chainId: number, tokenListPath: string | undefined): Array<Token> { | ||
export function getTokensFromTokenList(chainId: number, tokenListPath: string | undefined): Array<Token> { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @alfetopito , i believe this method should also filter the tokens by I feel unsure if you omitted this on porpoise to discover tokens that are not listed for that specific chain but share the same address as one that is listed As im unsure, i will play safe here and will NOT change this behaviour. Please, consider doing the filter when you are back There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. You are right, it could as well filter tokens here. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Pk, thanks for checking |
||
const filePath = tokenListPath | ||
? tokenListPath | ||
: join(BASE_PATH, chainId === 5 ? 'CowSwapGoerli.json' : 'CowSwap.json') | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,17 @@ | ||
import { isSupportedPermitInfo, PermitInfo } from '@cowprotocol/permit-utils' | ||
import { Token } from '../types.js' | ||
|
||
export function getUnsupportedTokensFromPermitInfo( | ||
chainId: number, | ||
allPermitInfo: Record<string, PermitInfo>, | ||
): Token[] { | ||
const tokens = [] | ||
|
||
for (const [k, v] of Object.entries(allPermitInfo)) { | ||
if (!isSupportedPermitInfo(v)) { | ||
tokens.push({ address: k, name: v?.name, chainId }) | ||
} | ||
} | ||
|
||
return tokens | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
New helpers to recheck the ones that were previously marked as unsupported