search

trustwallet / wallet-core

Cross-platform, cross-blockchain wallet library.
https://developer.trustwallet.com/wallet-core
Apache License 2.0
2.81k stars 1.58k forks source link

getKeyBIP44(..) for bitcoin coinType is incorrect #1378

Closed iamgenchi closed 3 years ago

iamgenchi commented 3 years ago

Describe the bug getKeyBIP44(..) for a .bitcoin coinType is incorrect and is using the BIP88 purpose instead.

To Reproduce

let wallet = HDWallet(mnemonic: "swarm swap wagon attitude roof inquiry all file reform nose put census", passphrase: "")
let coin: CoinType = .bitcoin

print(wallet.getKey(coin: coin, derivationPath: "m/44'/0'/0'/0/0").data.hexString)
// CORRECT: 8b1ddc92d9efff6dfc80b848c585258afb23098918cfe10a5f5545e7ec515a89

let key = wallet.getKeyBIP44(coin: coin, account: 0, change: 0, address: 0)
print(key.data.hexString)
// INCORRECT: 55a726045fc154d91e6b94b0f990b85322ec93cadce843cfb7b266c0da1d2a46

// This looks more like getKeyBIP44 actually is getKeyBIP84
print(wallet.getKey(coin: coin, derivationPath: "m/84'/0'/0'/0/0").data.hexString)
// 55a726045fc154d91e6b94b0f990b85322ec93cadce843cfb7b266c0da1d2a46

Expected behavior

getKeyBIP44() should return the private key using Bip44 purpose, not Bip84 purpose.

hewigovens commented 3 years ago

it's not a real bug, the API name is a bit misleading, it gets purpose from coins.json which is 84. we will improve it later

iamgenchi commented 3 years ago

A bit misleading indeed! Ok, how should we rename it?

optout21 commented 3 years ago

getDerivedKey

iamgenchi commented 3 years ago

Amazing @hewigovens.. Thanks heaps :pray: