-
Notifications
You must be signed in to change notification settings - Fork 67
chore: update swaps client interfaces #893
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
base: feat/swaps-client
Are you sure you want to change the base?
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
Summary of ChangesHello @slavastartsev, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request focuses on expanding the project's token support by integrating the BNB token across key blockchain networks, specifically Ethereum and BNB Smart Chain. The changes include adding comprehensive metadata and visual assets for BNB, alongside a crucial correction to prevent misidentification of ETH on BNB Smart Chain. This ensures that applications consuming these token lists will have accurate and up-to-date information for BNB. Highlights
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
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.
Code Review
This pull request correctly refactors the token list to add BNB as a distinct token, separating it from the previous ETH-based workaround. This includes adding a new data file for BNB and updating the main token lists. However, there is a critical issue with the new BNB logo file, which currently contains the Ethereum logo. This needs to be fixed to avoid user confusion.
tokenlist/data/BNB/logo.svg
Outdated
| <svg xmlns="http://www.w3.org/2000/svg" width="2500" height="2500" viewBox="0 0 32 32"> | ||
| <g fill="none" fill-rule="evenodd"> | ||
| <circle cx="16" cy="16" r="16" fill="#627EEA"/> | ||
| <g fill="#FFF" fill-rule="nonzero"> | ||
| <path fill-opacity=".602" d="M16.498 4v8.87l7.497 3.35z"/> | ||
| <path d="M16.498 4L9 16.22l7.498-3.35z"/> | ||
| <path fill-opacity=".602" d="M16.498 21.968v6.027L24 17.616z"/> | ||
| <path d="M16.498 27.995v-6.028L9 17.616z"/> | ||
| <path fill-opacity=".2" d="M16.498 20.573l7.497-4.353-7.497-3.348z"/> | ||
| <path fill-opacity=".602" d="M9 16.22l7.498 4.353v-7.701z"/> | ||
| </g> | ||
| </g> | ||
| </svg> No newline at end of file |
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.
The provided SVG file for the BNB logo is incorrect. It actually contains the logo for Ethereum. This is evident from:
- The
fillcolor#627EEA, which is Ethereum's brand color. BNB's color is typically yellow/gold (e.g.,#F0B90B). - The
<path>data, which draws the Ethereum octahedron shape, not the BNB diamond.
Using the wrong logo can be very misleading for users. Please replace this with the correct SVG for the BNB token.
417f239 to
dfed2b8
Compare
No description provided.