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.

Sign up for GitHub

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

Jump to bottom

[sdk/python] BIP 44 coin id should depends on network #241

Closed
cryptoBeliever opened this issue May 30, 2022 · 0 comments · Fixed by #247
Closed

[sdk/python] BIP 44 coin id should depends on network #241

cryptoBeliever opened this issue May 30, 2022 · 0 comments · Fixed by #247
Assignees
@Jaguar0625 @gimre-xymcity
Labels
sdk-python Related to the Python SDK. Status: WIP This issue or PR is a work in progress.

Comments

@cryptoBeliever
Copy link
Contributor

cryptoBeliever commented May 30, 2022 •
edited

Current Behaviour

Currently, SymbolFacade/NemFacade holds a static value of BIP coin id:

https://github.com/symbol/symbol/blob/dev/sdk/python/symbolchain/facade/SymbolFacade.py#L30
https://github.com/symbol/symbol/blob/dev/sdk/python/symbolchain/facade/NemFacade.py#L13

What's the problem?

BIP coin id should depend on the network. Currently, in Facades, we have values for mainnet coin id (43 for NEM and 4343 for Symbol). This is invalid in the case of testnet where coin id should be 1 (as is currently used by wallets and how BIP 44 specification tells https://github.com/satoshilabs/slips/blob/master/slip-0044.md)

Expected Behaviour

For all non-mainnet networks, we should use BIP 44 coin id = 1 as described in BIP 44 specification.

The corresponding issue for JS SDK: #242
@cryptoBeliever cryptoBeliever added Status: WIP This issue or PR is a work in progress. sdk-python Related to the Python SDK. labels May 30, 2022
@cryptoBeliever cryptoBeliever mentioned this issue May 30, 2022
Closed
Jaguar0625 added a commit that referenced this issue May 30, 2022
@Jaguar0625
[sdk]: replace BIP32_COIN_ID constant with network-dependent bip32Path
4cfc42f 
 problem: BIP32_COIN_ID constant contains mainnet coin id.
          This can be confusing when using testnet where a different coin id should be used.
solution: Drop BIP32_COIN_ID and replace it with a bip32Path function that returns a BIP32 path
          given an account id.

  issues: #241, #242
@Jaguar0625 Jaguar0625 mentioned this issue May 30, 2022
Merged
@Jaguar0625 Jaguar0625 linked a pull request May 30, 2022 that will close this issue
[sdk]: replace BIP32_COIN_ID constant with network-dependent bip32Path #247
Merged
Jaguar0625 added a commit that referenced this issue May 30, 2022
@Jaguar0625
[sdk]: replace BIP32_COIN_ID constant with network-dependent bip32Path
0289906 
 problem: BIP32_COIN_ID constant contains mainnet coin id.
          This can be confusing when using testnet where a different coin id should be used.
solution: Drop BIP32_COIN_ID and replace it with a bip32Path function that returns a BIP32 path
          given an account id.

  issues: #241, #242
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@Jaguar0625 Jaguar0625

@gimre-xymcity gimre-xymcity

Labels
sdk-python Related to the Python SDK. Status: WIP This issue or PR is a work in progress.
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

[sdk]: replace BIP32_COIN_ID constant with network-dependent bip32Path symbol/symbol
3 participants
@Jaguar0625 @gimre-xymcity @cryptoBeliever