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.

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

Merged
merged 22 commits into from
Dec 7, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
22 commits
Select commit Hold shift + click to select a range
e00399f
feat: update schema
alfetopito Nov 3, 2023
cede9da
chore: update unit tests to match new schema
alfetopito Nov 3, 2023
079b90f
refactor: rename getTokens to getTokensFromTokenList
alfetopito Nov 9, 2023
1336fa4
feat: update all permits to new format
alfetopito Nov 9, 2023
18c8268
feat: add option to recheck unsupported tokens
alfetopito Nov 9, 2023
cf728ac
chore: do not use name from token list
alfetopito Nov 9, 2023
64a72d2
refactor: simplify conditional
alfetopito Nov 9, 2023
ee190d9
feat: update sort fn
alfetopito Nov 9, 2023
4dade58
chore: make name and symbol optional
alfetopito Nov 9, 2023
2f8d2ce
chore: fix conditional
alfetopito Nov 9, 2023
cb0a127
refactor: move getUnsupportedTokensFromPermitInfo to utils fn
alfetopito Nov 9, 2023
ef56db8
chore: use p-throttle for limiting how often the permit check is done…
alfetopito Nov 9, 2023
21e345c
chore: remove unnecessary optional operators
alfetopito Nov 10, 2023
505aa38
chore: update to latest permit-utils, which doesn't work as it's not …
alfetopito Nov 10, 2023
dcd9dc7
chore: 4 more permittable tokens found...
alfetopito Nov 14, 2023
814c5fd
chore: 2 more tokens...
alfetopito Nov 14, 2023
015b755
chore: add p-retry for retrying :)
alfetopito Nov 14, 2023
c63455e
chore: retry on exceptions and connection errors
alfetopito Nov 14, 2023
661c9cd
chore: also retry on rpc connection failures
alfetopito Nov 14, 2023
8107d7b
feat: use published version of @cowprotocol/permit-utils
alfetopito Nov 17, 2023
868f4e2
chore: ran against https://tokens.honeyswap.org list
alfetopito Nov 17, 2023
ccf93e3
Set the original name of the contracts: result from running the script
anxolin Dec 7, 2023
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
11 changes: 8 additions & 3 deletions package.json
Original file line number Diff line number Diff line change
Expand Up @@ -15,20 +15,25 @@
"fetchPermitInfo:mainnet": "ts-node src/permitInfo/fetchPermitInfo.ts 1",
"fetchPermitInfo:gnosis": "ts-node src/permitInfo/fetchPermitInfo.ts 100",
"fetchPermitInfo:goerli": "ts-node src/permitInfo/fetchPermitInfo.ts 5",
"recheckPermitInfo:mainnet": "ts-node src/permitInfo/fetchPermitInfo.ts 1 '' '' true",
"recheckPermitInfo:gnosis": "ts-node src/permitInfo/fetchPermitInfo.ts 100 '' '' true",
"recheckPermitInfo:goerli": "ts-node src/permitInfo/fetchPermitInfo.ts 5 '' '' true",
Comment on lines +18 to +20
Copy link
Collaborator Author

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

"test": "node --test"
},
"license": "(MIT OR Apache-2.0)",
"dependencies": {
"@cowprotocol/permit-utils": "0.0.1-RC.1",
"@cowprotocol/permit-utils": "^0.0.1",
"@uniswap/token-lists": "^1.0.0-beta.33",
"ajv": "^8.12.0",
"ajv-cli": "^5.0.0",
"ajv-formats": "^2.1.1",
"axios": "^1.0.0",
"ts-node": "^10.9.1",
"exponential-backoff": "^3.1.1",
"lodash": "^4.17.21",
"node-fetch": "^3.3.0"
"node-fetch": "^3.3.0",
"p-retry": "^6.1.0",
"p-throttle": "^5.1.0",
"ts-node": "^10.9.1"
},
"devDependencies": {
"@types/node": "^20.8.7",
Expand Down
88 changes: 60 additions & 28 deletions src/permitInfo/fetchPermitInfo.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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'
Expand All @@ -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.')
Expand All @@ -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`)
Expand All @@ -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)
}
})

Expand All @@ -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
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Throttling the call to try and avoid RPC failures.
Still, every time I run the recheck, new tokens are identified as permittable.

Copy link
Contributor

Choose a reason for hiding this comment

The 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)
Is not 2 per second too slow?
What do you mean with "Still, every time I run the recheck, new tokens are identified as permittable." is the script never finishing?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Ran it today again to get an example:

image

Will see how I can mark those failures as retryable

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Made the script more robust, and added retries.
Still found some new permittable tokens, but not after the retries were introduced.


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 🏁`))
49 changes: 23 additions & 26 deletions src/permitInfo/permitInfo.schema.json
Original file line number Diff line number Diff line change
Expand Up @@ -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
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

No longer have the type with only false.

"name": {
"type": "string",
"description": "Token name as defined in the contract"
Comment on lines +27 to +29
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Added name

}
},
"required": [
"type",
"name"
]
}
},
Expand Down
41 changes: 39 additions & 2 deletions src/permitInfo/permitInfo.schema.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -13,9 +13,12 @@ describe('The permitInfo schema', () => {
})

describe('Valid PermitInfo data', () => {
it('should be valid with `false` value', () => {
it('should be valid with `unsupported` type', () => {
const data = {
'0x0000000000000000000000000000000000000000': false,
'0x0000000000000000000000000000000000000000': {
type: 'unsupported',
name: 'tokenName'
},
}

const ajv = new Ajv()
Expand All @@ -29,6 +32,7 @@ describe('Valid PermitInfo data', () => {
const data = {
'0x0000000000000000000000000000000000000000': {
type: 'eip-2612',
name: 'tokenName'
},
}

Expand All @@ -44,6 +48,7 @@ describe('Valid PermitInfo data', () => {
'0x0000000000000000000000000000000000000000': {
type: 'eip-2612',
version: '1',
name: 'tokenName'
},
}

Expand All @@ -58,6 +63,7 @@ describe('Valid PermitInfo data', () => {
const data = {
'0x0000000000000000000000000000000000000000': {
type: 'dai-like',
name: 'tokenName'
},
}

Expand All @@ -73,6 +79,7 @@ describe('Valid PermitInfo data', () => {
'0x0000000000000000000000000000000000000000': {
type: 'dai-like',
version: '2',
name: 'tokenName'
},
}

Expand All @@ -90,6 +97,7 @@ describe('Invalid PermitInfo data', () => {
'0x0000000000000000000000000000000000000000': {
type: 'eip-2612',
version: 1,
name: 'tokenName'
},
}

Expand All @@ -105,6 +113,21 @@ describe('Invalid PermitInfo data', () => {
'0x0000000000000000000000000000000000000000': {
type: 'eip-2612',
version: '1.1',
name: 'tokenName'
},
}

const ajv = new Ajv()
const result = ajv.validate(schema, data)

assert.strictEqual(result, false)
assert.notEqual(ajv.errors, null)
})

it('should be invalid without `name`', () => {
const data = {
'0x0000000000000000000000000000000000000000': {
type: 'eip-2612',
},
}

Expand Down Expand Up @@ -143,6 +166,7 @@ describe('Invalid PermitInfo data', () => {
const data = {
'0x0000000000000000000000000000000000000000': {
type: 'non-existent',
name: 'tokenName'
},
}

Expand All @@ -152,4 +176,17 @@ describe('Invalid PermitInfo data', () => {
assert.strictEqual(result, false)
assert.notEqual(ajv.errors, null)
})


it('should be invalid with `false` value', () => {
const data = {
'0x0000000000000000000000000000000000000000': false,
}

const ajv = new Ajv()
const result = ajv.validate(schema, data)

assert.strictEqual(result, false)
assert.notEqual(ajv.errors, null)
})
})
4 changes: 2 additions & 2 deletions src/permitInfo/types.ts
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
Expand Up @@ -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> {
Copy link
Contributor

Choose a reason for hiding this comment

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

@alfetopito , i believe this method should also filter the tokens by chainId.

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

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

You are right, it could as well filter tokens here.
Not a big problem though as they are filtered out later in the script.
This was meant as a simple fn to load tokens from a token list, taking into account that Goerli as a list of its own.

Copy link
Contributor

Choose a reason for hiding this comment

The 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')
Expand Down
17 changes: 17 additions & 0 deletions src/permitInfo/utils/getUnsupportedTokensFromPermitInfo.ts
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
}
Loading
Loading