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

Add Curve Tokenlists #4894

Merged
merged 7 commits into from
Sep 26, 2024
Merged

Add Curve Tokenlists #4894

merged 7 commits into from
Sep 26, 2024

Conversation

bout3fiddy
Copy link
Contributor

Add curve tokenlists for ethereum, arbitrum and gnosis

Summary

Adds Curve tokenlists to cowswap's frontend. At the moment i've specified them to be set by default (why would i want anything else?). but we can debate this no problems.

The tokenlists are generated via: https://github.com/curvefi/curve-assets
the flow is:

  1. user does a pull request with a .png file and a lowercase address to the folder containing token logos for a specific network
  2. on pull requests, a cicd pipeline runs and only generates a tokenlist if the token actually exists. else it will fail.
  3. cicd ensures random tokens cannot be added since the pipeline does rpc calls to check name, symbol and decimals.

To Test

go to urls specified:

Ethereum tokenlist
Arbitrum tokenlist
Gnosis tokenlist

Background

I'd like tigher integration between Curve markets and cowswap and this is one step along that direction.

Add curve tokenlists for ethereum, arbitrum and gnosis
Copy link

vercel bot commented Sep 15, 2024

@bout3fiddy is attempting to deploy a commit to the cow Team on Vercel.

A member of the Team first needs to authorize it.

Copy link
Contributor

github-actions bot commented Sep 15, 2024

CLA Assistant Lite bot:
Thank you for your submission, we really appreciate it. Like many open-source projects, we ask that you all sign our Contributor License Agreement before we can accept your contribution. You can sign the CLA by just posting a Pull Request Comment same as the below format.


I have read the CLA Document and I hereby sign the CLA


1 out of 2 committers have signed the CLA.
@shoom3301
@bout3fiddy
You can retrigger this bot by commenting recheck in this Pull Request

on request, removed 'enabledByDefault'
Copy link

vercel bot commented Sep 16, 2024

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

Name Status Preview Updated (UTC)
swap-dev ✅ Ready (Inspect) Visit Preview Sep 26, 2024 5:57am

Copy link
Contributor

@elena-zh elena-zh left a comment

Choose a reason for hiding this comment

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

Hey @bout3fiddy , thank you for adding token lists!
However, I don't see these lists added to Ethereum and Arbitrum networks, only to Gnosis. Could you please check this?
image

@shoom3301
Copy link
Collaborator

@elena-zh does it work in incognito mode?

@elena-zh
Copy link
Contributor

@elena-zh does it work in incognito mode?

No

Copy link
Collaborator

@alfetopito alfetopito left a comment

Choose a reason for hiding this comment

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

Looking at the list code, I see that all versions are set to 1.0.0.
Is this updated when the list is updated?

If not, it'll cause issues, since it relies on that to identify when the list should be updated or not.

See the reference https://github.com/Uniswap/token-lists?tab=readme-ov-file#semantic-versioning

@shoom3301
Copy link
Collaborator

There is an error in the list:

failed to process list https://curvefi.github.io/curve-assets/ethereum.json Error: Token list failed validation: .tokens[1].name should NOT be longer than 40 characters; .tokens[328].name should NOT be longer than 40 characters; .tokenMap['1_0xca2a7068e551d5c4482eb34880b194e4b945712f'].name should NOT be longer than 40 characters; .tokenMap['1_0xfbb481a443382416357fa81f16db5a725dc6cec8'].name should NOT be longer than 40 characters
    at validateTokenList (validateTokenList.ts:56:9)

@elena-zh p.s. you can find it in the console logs

@shoom3301
Copy link
Collaborator

So, the problem in the token with name Mercury RWA Token, 1:10,000 (Borderless.Fi), it has a 43 chars length

@shoom3301
Copy link
Collaborator

p.s. we use this schema for lists validation

@bout3fiddy
Copy link
Contributor Author

Hi! Just having a look at these. I'll make the required changes and submit. Weird token name. I'll get to the version updating as well.

@bout3fiddy
Copy link
Contributor Author

bout3fiddy commented Sep 24, 2024

So, the problem in the token with name Mercury RWA Token, 1:10,000 (Borderless.Fi), it has a 43 chars length

@shoom3301 I see that the Name has a max length of 60 and not 40?

https://github.com/Uniswap/token-lists/blob/f57d6be149b47a69c62fd3b3f77d8594b5fdf653/src/tokenlist.schema.json#L235

@shoom3301
Copy link
Collaborator

@bout3fiddy hi!
We just simplfied the token list validation, please, pull the latest changes from develop branch

bout3fiddy and others added 4 commits September 25, 2024 12:09
Add curve tokenlists for ethereum, arbitrum and gnosis
on request, removed 'enabledByDefault'
@shoom3301
Copy link
Collaborator

@elena-zh The PR was updated, now everything is great

image

Copy link
Contributor

@elena-zh elena-zh left a comment

Choose a reason for hiding this comment

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

Looks good to me, thank you!

@shoom3301 shoom3301 merged commit 69ec27c into cowprotocol:develop Sep 26, 2024
6 of 12 checks passed
@github-actions github-actions bot locked and limited conversation to collaborators Sep 26, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants