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

Fix ethereum-based chains testnet public keys #246

Merged
merged 1 commit into from
Sep 4, 2024
Merged

Conversation

haider-rs
Copy link
Contributor

@haider-rs haider-rs commented Jun 25, 2024

Description

This PR fixes ethereum based chains testnet public keys which are different then what public wallet shows.
It changes coin ids from 1 to 60 for current ethereum based supported testnets by chain-connectors.

Fixes # (issue)
#237

Type of change

  • Bug fix (non-breaking change which fixes an issue)
  • Breaking change (fix or feature that would cause existing functionality to not work as expected)

Tests

Describe the tests that you ran to verify your changes. Please also list any relevant details for your test configuration

  • Imported crate and verified a mnemonic's public key with public key of metamask

Code review prechecks:

  • Code follows the style guidelines of this project
  • Code has been self-reviewed

@haider-rs haider-rs linked an issue Jun 25, 2024 that may be closed by this pull request
@haider-rs haider-rs self-assigned this Jun 25, 2024
@haider-rs haider-rs requested review from Lohann and dvc94ch June 25, 2024 10:20
@haider-rs haider-rs force-pushed the fix-testnet-coin-id branch from afb9b1d to 290e23a Compare July 5, 2024 12:42
Copy link
Collaborator

@Lohann Lohann left a comment

Choose a reason for hiding this comment

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

Waiting for @penumbra23 thumps up

@dvc94ch dvc94ch merged commit f472ac3 into master Sep 4, 2024
8 checks passed
@dvc94ch dvc94ch deleted the fix-testnet-coin-id branch September 4, 2024 11:17
@FlorianFranzen
Copy link

@Haider-Ali-DS @Lohann Should we "fix" Astar as well, if metamask uses a 60 for them as well? Are there other wallets that use the correct coin identifier?

@Lohann
Copy link
Collaborator

Lohann commented Sep 6, 2024

The issue is that we also use the BIP44 ID for Astar SS58 address, the issues is that if you import a mneumonic on Metamask, it will derivate the wallet from 60, if you import it on PolkadotJS extension, it will use a different derivation.

@FlorianFranzen FlorianFranzen changed the title #237 Fix ethereum-based chains testnet public keys Fix ethereum-based chains testnet public keys Sep 10, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Different public keys from different wallets
4 participants