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 rootstock chain #3205

Merged
merged 1 commit into from
Jun 9, 2023
Merged

Add rootstock chain #3205

merged 1 commit into from
Jun 9, 2023

Conversation

ahsan-javaiid
Copy link
Contributor

Description

Adds Support for rootstock chain.

How to test

Same way as other EVM chains

Types of changes

Checklist

  • Create pull request as draft initially, unless its complete.
  • Add tests to cover changes as needed.
  • Update documentation as needed.
  • If there is a related Issue, mention it in the description.

If you're adding a new blockchain

  • I have read the guidelines for adding a new blockchain.

@ahsan-javaiid
Copy link
Contributor Author

cc: @alepc253

@Milerius
Copy link
Collaborator

Milerius commented Jun 8, 2023

Hey, please add your network using custom network feature on mobile.

@Milerius Milerius closed this Jun 8, 2023
@ahsan-javaiid
Copy link
Contributor Author

Hi @Milerius

I created this pull request while doing integration with alpha-wallet which uses this wallet-core sdk. And rootstock is not supported by wallet-core due to which integration will be incomplete.

I added rootstock as custom network but still we need this integration to support the rootstock coin type. So that wallet could use the correct derivation path here

The purpose of this pull request is not only to list rootstock network but to support the coin type as there are many other wallets using this sdk.

I request to reconsider this pull request as it will open doors for rootstock network integration with multiple wallets using this sdk. 🙏

@Milerius Milerius reopened this Jun 8, 2023
registry.json Show resolved Hide resolved
@Milerius
Copy link
Collaborator

Milerius commented Jun 8, 2023

It's seems that tests are missing in CoinAddressDerivationTests kotlin/swift can you please fix @ahsan-javaiid

@trustwallet trustwallet deleted a comment Jun 8, 2023
@trustwallet trustwallet deleted a comment Jun 8, 2023
@ahsan-javaiid
Copy link
Contributor Author

It's seems that tests are missing in CoinAddressDerivationTests kotlin/swift can you please fix @ahsan-javaiid

Added the tests. Thanks for pointing it out. 🙏

Copy link
Collaborator

@satoshiotomakan satoshiotomakan left a comment

Choose a reason for hiding this comment

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

LGTM

@Milerius Milerius merged commit 2b289fa into trustwallet:master Jun 9, 2023
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.

3 participants