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

Python3 Bug prevents signing multisig txs #18

Closed
kyuupichan opened this issue Nov 8, 2017 · 5 comments
Closed

Python3 Bug prevents signing multisig txs #18

kyuupichan opened this issue Nov 8, 2017 · 5 comments

Comments

@kyuupichan
Copy link
Contributor

Problem is simple to reproduce:

import keepkeylib.ckd_public as bip32
bip32.deserialize("xpub6DF8uhdarytz3FWdA8TvFSvvAh8dP3283MY7p2V4SeE2wyWmG5mg5EwVvmdMVCQcoNJxGoWaU9DCWh89LojfZ537wTfunKau47EL2dhHKon")
Traceback (most recent call last):
File "", line 1, in
File "/usr/local/lib/python3.6/site-packages/keepkeylib/ckd_public.py", line 111, in deserialize
raise Exception("Checksum failed")
Exception: Checksum failed

The bug is tools.py/b58decode getting confused between strings and binary types. I suggest replacing with the trezorlib implementation that I've verified works fine.

That is unlikely to be the only function in that file with such a bug; please add tests for them

@kyuupichan
Copy link
Contributor Author

By the way, the above xpub is taken from BIP32 specification document

@nickmonad
Copy link
Contributor

@kyuupichan I've pushed up a branch and verified the above function works in both python2 and python3. Please pull down this branch and test when you get a chance. Working on adding unit tests.

@kyuupichan
Copy link
Contributor Author

@ngmiller I've checked out the branch and things seem fine. Multisig txs are signed fine in Electron Cash but as I point out elsewhere there is a SIGHASH_FORKID problem when sending it. I've not had time to examine in detail, but my suspicion is a firmware bug. It could be an Electron Cash bug too though. If you want to know how to reproduce that I can write an explanation.

@kyuupichan
Copy link
Contributor Author

Are you certain multisig txs are being signed with SIGHASH_FORKID when the coin is BCH?

@nickmonad
Copy link
Contributor

@kyuupichan We are looking into the SIGHASH_FORKID as a separate issue, since the specific function you mentioned in this issue has been addressed. I am working on this today.

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

No branches or pull requests

2 participants