Bit-Wasp / bitcoin-php

Bitcoin implementation in PHP
The Unlicense
1.05k stars 416 forks source link

Failed to extract HierarchicalKey from parser #854

Open panoroman opened 4 years ago

panoroman commented 4 years ago

Hello, your lib is awesome, but i found problem. On every 1000 generated wallets by mnemonic, 6 wallets is invalid. However, on the node.js, these mnemonics generate valid wallets. I suppose, that because length of string 77 bytes and not 78, and nodejs encodes the mnemonic differently. This error comes from ../Parser.php with throw new ParserOutOfRange('Could not parse string of required length (too short)').

panoroman commented 4 years ago

And these mnemonics

uphold march clog swift issue twelve hazard defy buzz exercise syrup flip twenty bunker dutch ridge east marble cricket future flag error fruit chest

renew human armed stage actual thank lumber fault team same second door journey dragon bridge monster explain very airport add endorse erupt visa energy

either upper train super leisure anger any divide reduce flavor electric unusual various virus cabin swarm space example just salmon purity february black symbol

napkin weapon income wine bring effort jump parrot future diesel damage alpha kit immune recycle sing genuine suit base dice option uphold evil myth

crowd debate famous whisper liar tray boat learn hint later grace process any concert cost spin frame garlic exact media spawn humor alone joy

roof sudden traffic young clutch universe december village favorite rally tissue clog soon rally fossil squirrel absent wisdom inner tomorrow twenty tortoise same source

afk11 commented 4 years ago

Oh wow. this is interesting, thanks for for the report. Do you have sample code of what you were doing when you got ParserOutOfRange?

panoroman commented 4 years ago

Yes, of course. In this project https://bitbucket.org/decimalteam/decimal-php-sdk/src/master/, after create instance $wallet = new Wallet(/your mnemonics/);

afk11 commented 4 years ago

Just thinking about it, you shouldn't have to parse a hierarchical key when deriving from a mnemonic?

I'll try find time to look into this. It's also polite to give code snippets from this library that reproduce the case ;) Is the decimal-php-sdk your project or just something you're using?

afk11 commented 4 years ago

I took a quick look - it looks like that project reimplemented the extended key -> base58 encoding themselves, and when the result is passed to bitcoin-php it rejects it as invalid. It looks like you'll need to ask them why.

(I dumped the 'extended key' it tried to decode, and unless I'm missing something, it's using an altcoin prefix, or the encoding is wrong: DeaWiTqmhSgUGkoBCLcQZQNuXpNj1aBnTzXMfrdWmE4feh3q81pacLGBBpRbcMMKAcRgtHyWUqBgP9SpjTysTnCkHx2v9Fhg2BjPCWopPyiy6z) https://bitbucket.org/decimalteam/decimal-php-sdk/src/d753d5c84083934cd75e58a528ead51071b4c35d/src/Utils/Crypto/Bip44/KeyPairs.php#lines-227

It's really strange how it mixes 2/3 crypto/bitcoin libraries together, all of this is implemented + tested in bitcoin-php anyway :)

panoroman commented 4 years ago

Thank you for advise ) I know why this project use crypto libraries together bitcoin, this is for create ethereum wallet on derivation path ("m/44'/60'/0'/0"). Can bitcoin-php itself create wallet on this derivation path ("m/44'/60'/0'/0") ?

afk11 commented 4 years ago

Ah I see. Hmm, yea it can do bip32! Assuming hdKey is the root key you'd do

$hdKey->derivePath("44'/80'/0'/0'")

These might help https://github.com/Bit-Wasp/bitcoin-php/blob/1.0/examples/bip32.php https://github.com/Bit-Wasp/bitcoin-php/blob/1.0/examples/trezor.bip32.php