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 and update PulseChain Networks #626

Merged
merged 1 commit into from
Sep 12, 2024

Conversation

bretep
Copy link
Contributor

@bretep bretep commented Aug 12, 2024

Description

Please provide a detailed description of what was done in this PR.
(And mentioned if linked to an issue docs)

Changes include

  • Bugfix (non-breaking change that solves an issue)
  • New feature (non-breaking change that adds functionality)
  • Breaking change (change that is not backwards-compatible and/or changes current functionality)
  • Tests
  • Documentation
  • Other (for changes that might not fit in any category)

Additional comments

Support for PulseChain was missed during updates earlier this year. I've added it back following the new patterns.

Ledger Live integration: LedgerHQ/ledger-live#7607

@bretep bretep force-pushed the update-pulsechain branch 2 times, most recently from f1efae6 to fede1fd Compare August 23, 2024 18:16
@bretep bretep force-pushed the update-pulsechain branch 3 times, most recently from 26ed637 to 23e4e72 Compare September 6, 2024 15:05
@apaillier-ledger
Copy link
Contributor

Hello @bretep, thank you for this PR.

We now don't allow clone applications (with separate .mk files) that only use Ethereum's derivation path, since they will work just fine directly with the Ethereum app directly.

If you could update your PR by :

  • removing the two .mk files you added
  • removing the .gif files you added in icons/
  • since the glyphs for the mainnet and testnet are the same, to save space in both the repository and the app itself, create a symbolic link instead so that chain_943_64px.gif points to chain_369_64px.gif

Thank you! 🙏

@bretep
Copy link
Contributor Author

bretep commented Sep 10, 2024

@apaillier-ledger, Thank you for that excellent review and feedback.

I've done the following:

  • Removed the two .mk files
  • Removed the .gif icons located in ./icons
  • Symlinked the chain's .gif in ./glyphs to save space
  • Amended the previous commit to keep this a single commit
  • Force pushed

Please let me know if I need to fix anything else. I appreciate your help.

@bretep
Copy link
Contributor Author

bretep commented Sep 10, 2024

@apaillier-ledger I guess my only concern is DEFINES += HAVE_ETH2. Because PulseChain has a beacon chain and uses the same BLS curve as Ethereum, will validators still be able to use the Ethereum App to make validator addresses?

I'm guessing it will be fine, but I want to make sure we're not overlooking anything.

Most other Ethereum chains do not have a beacon chain; they use ETH1.

Reference:
https://beacon.pulsechain.com
https://launchpad.pulsechain.com

@apaillier-ledger
Copy link
Contributor

@bretep, it should behave with the Ethereum app the same way as it would have with the separate clone app.
Let us know if you have issues between your beacon chain and the Ethereum app, since the ETH2 support was initially developped for the mainnet one.

@apaillier-ledger apaillier-ledger merged commit 72aba0a into LedgerHQ:develop Sep 12, 2024
152 checks passed
@ixje ixje mentioned this pull request Sep 17, 2024
6 tasks
@bretep bretep deleted the update-pulsechain branch September 23, 2024 18:18
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants