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): fetch token name from chain #3374

Merged
merged 27 commits into from
Nov 17, 2023

Conversation

alfetopito
Copy link
Collaborator

Summary

Fetching token name from the contracts rather than relying on what comes from the token lists

Also, this PR changes the schema in 2 important ways:

  1. The fetched name is now stored
  2. No longer storing false values for unsupported. Instead, it'll have the type unsupported

A new version of the @cowprotocol/permit-utils should be published from these changes, so it can be used on token-lists repo.

To Test

  1. Test the regular permit stuff
  • Should work as it is, showing permit flags
  1. Import a permittable token not in any list, such as 0x0001a500a6b18995b03f44bb040a5ffc28e45cb0 on mainnet
  • Should be recognized and be tradeable with permit as expected

@alfetopito alfetopito self-assigned this Nov 9, 2023
Copy link

vercel bot commented Nov 9, 2023

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Comments Updated (UTC)
cosmos ✅ Ready (Inspect) Visit Preview 💬 Add feedback Nov 17, 2023 9:18am
swap-dev ✅ Ready (Inspect) Visit Preview Nov 17, 2023 9:18am
widget-configurator ✅ Ready (Inspect) Visit Preview 💬 Add feedback Nov 17, 2023 9:18am

Comment on lines +34 to +53
type OldPermitInfo = PermitInfo | false

const UNSUPPORTED: PermitInfo = { type: 'unsupported' }

/**
* Handles data migration from former way of storing unsupported tokens to the new one
*/
function migrateData(data: Record<string, OldPermitInfo>): Record<string, PermitInfo> {
const migrated: Record<string, PermitInfo> = {}

for (const [k, v] of Object.entries(data)) {
if (v === false) {
migrated[k] = UNSUPPORTED
} else {
migrated[k] = v
}
}

return migrated
}
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

This should only be needed until this hits prod and the token-lists preGenerated version is not updated.

Comment on lines -15 to +14
export const permittableTokensAtom = atomWithStorage<PermittableTokens>('permittableTokens:v1', {
export const permittableTokensAtom = atomWithStorage<PermittableTokens>('permittableTokens:v2', {
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Bumping the version to not use the old formats anymore

Comment on lines +1 to +16
[
{
"constant": true,
"inputs": [],
"name": "name",
"outputs": [
{
"name": "",
"type": "string"
}
],
"payable": false,
"stateMutability": "view",
"type": "function"
}
]
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

It's a duplication, I know, but I want to have just the minimal ABI possible.
Also, if I would use the one from the abis lib, it would create an inner dependency and I don't want that for this lib

Comment on lines +5 to -12
export type PermitType = 'dai-like' | 'eip-2612' | 'unsupported'

export type SupportedPermitInfo = {
export type PermitInfo = {
type: PermitType
version: string | undefined // Some tokens have it different than `1`, and won't work without it
// TODO: make it not optional once token-lists is migrated
name?: string
version?: string | undefined // Some tokens have it different than `1`, and won't work without it
}
type UnsupportedPermitInfo = false
export type PermitInfo = SupportedPermitInfo | UnsupportedPermitInfo
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Biggest change:

  1. Remove false version in favor of unsupported type
  2. Add name

Copy link

socket-security bot commented Nov 10, 2023

No top level dependency changes detected. Learn more about Socket for GitHub ↗︎

Copy link
Contributor

@anxolin anxolin left a comment

Choose a reason for hiding this comment

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

is it expected to not get the gass-free message?
image

@alfetopito
Copy link
Collaborator Author

alfetopito commented Nov 13, 2023

is it expected to not get the gass-free message? image

It should be. At least after it has been imported.
Did you try before or after importing it?

@anxolin
Copy link
Contributor

anxolin commented Nov 14, 2023

Did you try before or after importing it?

Both, the picture is after importing

@alfetopito
Copy link
Collaborator Author

Did you try before or after importing it?

Both, the picture is after importing

Hmm, not able to reproduce it.

I imported it via the address, and with this method I can see 2 ways if importing it.
And both show the right flag after imported.
Importing it by symbol also has the same behaviour.

image

Can you share the contents of your permittableTokens:v2 localStorage key?

image

It might be that your local check has failed and it cached the failure.

@alfetopito alfetopito merged commit 578df82 into develop Nov 17, 2023
9 of 10 checks passed
@alfetopito alfetopito deleted the permit/fetch-name-from-chain branch November 17, 2023 10:25
@github-actions github-actions bot locked and limited conversation to collaborators Nov 17, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants