-
Notifications
You must be signed in to change notification settings - Fork 333
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 PulseChain network #7607
Add PulseChain network #7607
Conversation
The latest updates on your projects. Learn more about Vercel for Git βοΈ
3 Skipped Deployments
|
Someone is attempting to deploy a commit to the LedgerHQ Team on Vercel. A member of the Team first needs to authorize it. |
I'm not able to see PulseChain app in the App catalog: I have installed LedgerHQ/app-ethereum#626, and it seems to work correctly when viewing and signing messages. Is there an additional step I need to take to add the PulseChain app, like the Binance Smart Chain (BNB) app? |
b087f83
to
5171503
Compare
16d8d4b
to
652d370
Compare
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.
I've tried to follow the published documentation and review patterns of the Polygon / Binance Smart Chain EVM networks.
Still unclear how to generate/sign the erc20 files, is this an internal process?
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.
It's unclear to me how to update this with the erc20s to be added. Please advise. There are about 50 tokens to be added here.
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.
I see that these files are all generated by importing the cryptoassets from CDN. Is there a bootstrapping process for an evm chain? We've got ~50 or so tokens to start off with. I've got a tokenlist that we can start from.
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.
I was testing the flow by adding the PulseX token using the Ledger developer guide on integrating EVM chain tokens. Unfortunately, I found that PulseChain isn't listed yet, which means we'll need to get PulseChain integrated before tokens can be submitted for review.
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.
Hi @bretep, thanks for your contribution :) Indeed, adding tokens list for new networks is an internal process, can you provide us with your list ?
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.
I've created a zip file containing the default tokens in JSON/YAML format. There are 16 tokens in total.
- 11 tokens are mainnet tokens and contain the website and coingecko id.
- 5 tokens are for the testnet, which are worthless.
I've decided not to include the extended tokenlist and allow each project to go through your process of onboarding tokens. If you advise you would like the extended tokenlist, I will be happy to provide, but we do not have the coingecko id or website for those tokens.
If you prefer I paste the tokens in the issue, what format would you like them pasted in?
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.
Thank you! Please let me know if I need to do anything else.
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.
@jnicoulaud-ledger, should I keep rebasing and force-pushing to keep up with the develop
branch?
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.
Hi, I pinged the relevant team to provide you with guidance on moving the PR forward
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.
Awesome! Thank you!
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.
652d370
to
75b36a8
Compare
75b36a8
to
98b6d42
Compare
98b6d42
to
1244638
Compare
cacd14b
to
bc35469
Compare
bc35469
to
aff0ad6
Compare
aff0ad6
to
9251a2d
Compare
@jnicoulaud-ledger I'm checking in to see the internal status of the PR. Is there something more I can do or another dev that can review this? Also, LedgerHQ/app-ethereum#626 needs review. |
Hi @bretep ! Thanks a lot for the PR ! For everything else (appearing in Ledger Live portfolio, adding tokens etc...), unfortunately you'll need to go through a process with our product department in order to get an approval first and be integrated in our roadmap. Sorry for the heavy process, but with the quantity of new chains we have to integrate, it became mandatory for us not to implode internally π Please join the Ledger Discord and ask about it there: https://discord.com/invite/ledger |
Thank you, @lambertkevin, for the additional information. Yes, I think we're on the same page; using the Ethereum App, this is what it should look like. :) I've been on Discord for some time now. Which channel and who should I reach out to? I've asked a couple of times on Discord but it seems like I'm not reaching the right people. Thank you, Bret |
Just to note that I've reached out here on Discord: https://ptb.discord.com/channels/885256081289379850/907623688759803935/1283079422525706342 |
9251a2d
to
5bd6c26
Compare
Hi @bretep, the correct channel is: #blockchain-foundation |
5bd6c26
to
c4408b0
Compare
There as been no activity on this PR for the last 14 days. Please consider closing this PR. |
c4408b0
to
6a359e1
Compare
6a359e1
to
108fb65
Compare
108fb65
to
9664f6c
Compare
Add tests for the PulseChain network Add changeset
9664f6c
to
487e07b
Compare
Why was this closed? |
β Checklist
npx changeset
was attached.π Description
Adds support for the PulseChain network.
Testing
I've tested this PR with LedgerHQ/app-ethereum#626 installed on:
Dependencies
This PR requires the changes introduced in LedgerHQ/app-ethereum#626.
π§ Checklist for the PR Reviewers