feat(permit): with token name #312

alfetopito · 2023-11-10T16:28:12Z

Summary

Addresses last part of #293

Depends on cowprotocol/cowswap#3374

⚠️ cowprotocol/cowswap#3374 must be deployed to PROD before this change can be merged!! ⚠️

✅ ~~Will not work until @cowprotocol/permit-utils is published from cowprotocol/cowswap#3374~~

Changes

Breaking schema change
Added name to PermitInfo
Added option to re-check unsupported tokens

Testing

Locally, I published @cowprotocol/permit-utils so I could run the script

With it, I updated all existing PermitInfo files

socket-security · 2023-11-10T16:28:54Z

New and updated dependencies detected. Learn more about Socket for GitHub ↗︎

Packages	Version	New capabilities	Transitives	Size	Publisher
p-retry	6.1.0	None	`+3`	43.1 kB	sindresorhus
p-throttle	5.1.0	None	`+0`	8.7 kB	sindresorhus
@cowprotocol/permit-utils	0.0.1-RC.1...0.0.1	None	`+0/-0`	26.5 kB	cowprotocol_dev

alfetopito · 2023-11-10T16:28:58Z

package.json

+    "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",


New helpers to recheck the ones that were previously marked as unsupported

alfetopito · 2023-11-10T16:29:47Z

src/permitInfo/fetchPermitInfo.ts

+// Fn can only be called 2x/second
+const throttle = pThrottle({
+  limit: 2,
+  interval: 1000,
+})
+
+const throttledGetTokenPermitInfo = throttle(getTokenPermitInfo)


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

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?

Ran it today again to get an example:

Will see how I can mark those failures as retryable

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

alfetopito · 2023-11-10T16:30:21Z

src/permitInfo/permitInfo.schema.json

-        {
-          "type": "boolean",
-          "description": "When a token is known to not be permittable",
-          "const": false


No longer have the type with only false.

alfetopito · 2023-11-10T16:30:31Z

src/permitInfo/permitInfo.schema.json

+        "name": {
+          "type": "string",
+          "description": "Token name as defined in the contract"


Added name

yarn.lock

alfetopito · 2023-11-10T16:32:55Z

src/public/PermitInfo.1.json

It got a bit verbose...
But look, so many new permittable tokens!!

anxolin

Format seems fine to me. Sure, a bit more verbose means more bytes to transfer, but probably don't makes a big difference, so is not worth complicating the interface.

I couldn't test the PR, but i trust you say it worked for you. I don't fully get the part where you say new permeable tokens are detected in each run.

anxolin · 2023-11-13T17:06:44Z

src/permitInfo/fetchPermitInfo.ts

+// Fn can only be called 2x/second
+const throttle = pThrottle({
+  limit: 2,
+  interval: 1000,
+})
+
+const throttledGetTokenPermitInfo = throttle(getTokenPermitInfo)


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?

yarn.lock

… and avoid rpc failures

…published yet

anxolin · 2023-12-07T12:08:38Z

src/permitInfo/utils/getTokensFromTokenList.ts

@@ -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> {


@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

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.

Pk, thanks for checking

alfetopito self-assigned this Nov 10, 2023

alfetopito commented Nov 10, 2023

View reviewed changes

alfetopito requested a review from a team November 10, 2023 17:32

anxolin reviewed Nov 13, 2023

View reviewed changes

alfetopito force-pushed the permit/with-token-name branch from 0e56c53 to a025b96 Compare November 14, 2023 17:07

alfetopito requested review from a team and anxolin November 17, 2023 11:27

alfetopito added 21 commits December 7, 2023 11:21

feat: update schema

e00399f

chore: update unit tests to match new schema

cede9da

refactor: rename getTokens to getTokensFromTokenList

079b90f

feat: update all permits to new format

1336fa4

feat: add option to recheck unsupported tokens

18c8268

chore: do not use name from token list

cf728ac

refactor: simplify conditional

64a72d2

feat: update sort fn

ee190d9

chore: make name and symbol optional

4dade58

chore: fix conditional

2f8d2ce

refactor: move getUnsupportedTokensFromPermitInfo to utils fn

cb0a127

chore: use p-throttle for limiting how often the permit check is done…

ef56db8

… and avoid rpc failures

chore: remove unnecessary optional operators

21e345c

chore: update to latest permit-utils, which doesn't work as it's not …

505aa38

…published yet

chore: 4 more permittable tokens found...

dcd9dc7

chore: 2 more tokens...

814c5fd

chore: add p-retry for retrying :)

015b755

chore: retry on exceptions and connection errors

c63455e

chore: also retry on rpc connection failures

661c9cd

feat: use published version of @cowprotocol/permit-utils

8107d7b

chore: ran against https://tokens.honeyswap.org list

868f4e2

anxolin force-pushed the permit/with-token-name branch from cf4c4bf to 868f4e2 Compare December 7, 2023 11:21

shoom3301 approved these changes Dec 7, 2023

View reviewed changes

anxolin marked this pull request as ready for review December 7, 2023 12:06

anxolin reviewed Dec 7, 2023

View reviewed changes

Set the original name of the contracts: result from running the script

ccf93e3

shoom3301 approved these changes Dec 7, 2023

View reviewed changes

anxolin self-requested a review December 7, 2023 14:25

anxolin approved these changes Dec 7, 2023

View reviewed changes

anxolin merged commit 8f10869 into main Dec 7, 2023
5 checks passed

anxolin deleted the permit/with-token-name branch December 7, 2023 14:25

github-actions bot locked and limited conversation to collaborators Dec 7, 2023

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat(permit): with token name #312

feat(permit): with token name #312

alfetopito commented Nov 10, 2023 •

edited

Loading

socket-security bot commented Nov 10, 2023 •

edited

Loading

alfetopito Nov 10, 2023

alfetopito Nov 10, 2023

anxolin Nov 13, 2023

alfetopito Nov 14, 2023

alfetopito Nov 14, 2023

alfetopito Nov 10, 2023

alfetopito Nov 10, 2023

alfetopito Nov 10, 2023

anxolin left a comment

anxolin Nov 13, 2023

anxolin Dec 7, 2023

alfetopito Dec 11, 2023

anxolin Dec 11, 2023

feat(permit): with token name #312

feat(permit): with token name #312

Conversation

alfetopito commented Nov 10, 2023 • edited Loading

Summary

Changes

Testing

socket-security bot commented Nov 10, 2023 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

anxolin left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

alfetopito commented Nov 10, 2023 •

edited

Loading

socket-security bot commented Nov 10, 2023 •

edited

Loading