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

fixes #39 - corrects signature_hash for SIGHASH_SINGLE on input index > 0; ... #40

Merged
merged 4 commits into from
Jun 5, 2014

Conversation

posita
Copy link
Contributor

@posita posita commented Jun 4, 2014

...adds ability to verify legacy SINGLE_HASH transactions; adds TxIn to tx/init.py for consistency with Tx and TxOut

Hopefully this fix for issue #39 helps redeem me from the utter embarrassment that is issue #38. :o(

I wrote a test script to compare the fixed output of pycoin with the equivalent output from bitcoinj. The python half is adapted as a (now working) unit test in e8b026f.

On a related note, your BRAIN DAMAGE: this probably doesn't work right comment was wrong...that part of the code was fine. It just required a number encoding tweak.... :o)

posita added 4 commits June 4, 2014 00:10
… > 0; adds ability to verify legacy SINGLE_HASH transactions; adds TxIn to tx/__init__.py for consistency with Tx and TxOut
…oin_streamer, but "fakes" the "null" value of -1L by using its unsigned equivalent instead)
@richardkiss
Copy link
Owner

Do you know of any test transactions in the blockchain that I can include in test cases?

richardkiss added a commit that referenced this pull request Jun 5, 2014
fixes #39 - corrects signature_hash for SIGHASH_SINGLE on input index > 0; ...
@richardkiss richardkiss merged commit 762fc89 into richardkiss:master Jun 5, 2014
@richardkiss
Copy link
Owner

Oh, I see you put some in, great! Thanks so much!

@posita
Copy link
Contributor Author

posita commented Jun 5, 2014

Yeah, those were contrived cases. These might help (as a battery of various transaction verifications):

https://github.com/bitcoin/bitcoin/blob/master/src/test/data/tx_valid.json#L39 (see also line 96)

I'd have to think about how to incorporate all of these into your unit tests. This may be beyond the scope of this pull request, and deserving of its own issue?

I have not independently verified this, but this bitcointalk.org post claims to cite three in-blockchain transactions with SIGHASH_SINGLE signatures where nIn > nOut (i.e., the old, buggy code path):

315ac7d4c26d69668129cc352851d9389b4a6868f1509c6c8b66bead11e2619f
dbf38261224ebff0c455c405e2435cfc69adb6b8a42d7b10674d9a4eb0464dca
de744408e4198c0a39310c8106d1830206e8d8a5392bcf715c9b5ec97d784edd

richardkiss added a commit that referenced this pull request Nov 24, 2014
ghtdak pushed a commit to ghtdak/pycoin that referenced this pull request Jul 16, 2016
fixes richardkiss#39 - corrects signature_hash for SIGHASH_SINGLE on input index > 0; ...
[gitreformat yapf-ify (github/ghtdak) on Fri Jul 15 12:50:46 2016]
[from commit: 762fc89]
ghtdak pushed a commit to ghtdak/pycoin that referenced this pull request Jul 16, 2016
[gitreformat yapf-ify (github/ghtdak) on Fri Jul 15 12:51:26 2016]
[from commit: c89f3d8]
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.

2 participants