petertodd / python-bitcoinlib

Python3 library providing an easy interface to the Bitcoin data structures and protocol.
Other
1.84k stars 624 forks source link

AssertionError when calculating SignatureHash #178

Open ghost opened 6 years ago

ghost commented 6 years ago

For example the transaction 858eb08ea0e24b052a5945270475af4e49f5b7158d9d5b6218bff53d1d9d53e9

from bitcoin.core import *
from bitcoin.core.script import *

raw_txid = '010000000190664219e4eedfc97020d7a7bacddaaf5abbe20e3f0a5e77e9ae85bcdf6a0fd0910000006f0047304402207bf94f0f703364bbc008fe814061a36f421a59153b49131b828e583802cb13670220301095b2cb26623840455155e97ed1831d05a50798128a21ae83e38e8bcd90680125512102494948b6192ca3615e443c10ec058497c2b6785c9c960940de15583930ad97c751aeffffffff01900e4d000000000017a914497d58d263bd82b460bdc9b8c69ca6e25acc99938700000000'
ctx = CTransaction().deserialize(bytes.fromhex(raw_txid))

publickey = '512102494948b6192ca3615e443c10ec058497c2b6785c9c960940de15583930ad97c751ae'
value = int(0.051*COIN)
cs = CScript(bytes.fromhex(publickey))

sighash = SignatureHash(cs, ctx, 0, SIGHASH_ALL, value, 0)
print(b2x(sighash))

results:

Traceback (most recent call last):
  File "/home/pc/Desktop/Pycharm/Transaction_Parser/test/test.py", line 11, in <module>
    sighash = SignatureHash(cs, ctx, 0, SIGHASH_ALL, value, 0)
  File "/home/pc/Desktop/Pycharm/venv/Ubuntu/lib/python3.6/site-packages/bitcoin/core/script.py", line 1018, in SignatureHash
    assert not script.is_witness_scriptpubkey()
AssertionError

Possible workaround is to comment out line 1018 assert not script.is_witness_scriptpubkey(), which then results the right SignatureHash '847b08415229a2a728f6a22077f2ad5c69faf6ae81d6c16f16e61425dc9a984f'.

dgpv commented 5 years ago

https://github.com/petertodd/python-bitcoinlib/pull/187

petertodd commented 5 years ago

Fixed now that #187 is merged?

dgpv commented 5 years ago
>>> from bitcoin.core import *
>>> from bitcoin.core.script import *
>>> 
>>> raw_txid = '010000000190664219e4eedfc97020d7a7bacddaaf5abbe20e3f0a5e77e9ae85bcdf6a0fd0910000006f0047304402207bf94f0f703364bbc008fe814061a36f421a59153b49131b828e583802cb13670220301095b2cb26623840455155e97ed1831d05a50798128a21ae83e38e8bcd90680125512102494948b6192ca3615e443c10ec058497c2b6785c9c960940de15583930ad97c751aeffffffff01900e4d000000000017a914497d58d263bd82b460bdc9b8c69ca6e25acc99938700000000'
>>> ctx = CTransaction().deserialize(bytes.fromhex(raw_txid))
>>> 
>>> publickey = '512102494948b6192ca3615e443c10ec058497c2b6785c9c960940de15583930ad97c751ae'
>>> value = int(0.051*COIN)
>>> cs = CScript(bytes.fromhex(publickey))
>>> 
>>> sighash = SignatureHash(cs, ctx, 0, SIGHASH_ALL, value, 0)
>>> print(b2x(sighash))
847b08415229a2a728f6a22077f2ad5c69faf6ae81d6c16f16e61425dc9a984f
>>> print(__version__)
0.11.0dev
dgpv commented 5 years ago

and on v0.10.2 the assertion still happens.

petertodd commented 5 years ago

@dgpv Wait, what do you mean by your code snippet above?

dgpv commented 5 years ago

I just took this code snippet from the first message, and run it on the last version of the library. It did not cause exception. Then I checked out v0.10.2, and run the same code snippet, and got the same assertion as in the first message. I just mean "I did a one-time test on this, it worked"

petertodd commented 5 years ago

@dgpv Ah! Well, if you figure out a fix, pull-req accepted, and I'll try to merge it in a more reasonable amount of time than your other (much appreciated!) segwit improvements. :)

dgpv commented 5 years ago

I mean, this shows that #187 fixed the issue.

The transaction in this issue contains 1-of-1 p2sh script:

>>> from bitcoin.core import *
>>> from bitcoin import *
>>> from bitcoin.core.script import *
>>> tx=CTransaction.deserialize(x('010000000190664219e4eedfc97020d7a7bacddaaf5abbe20e3f0a5e77e9ae85bcdf6a0fd0910000006f0047304402207bf94f0f703364bbc008fe814061a36f421a59153b49131b828e583802cb13670220301095b2cb26623840455155e97ed1831d05a50798128a21ae83e38e8bcd90680125512102494948b6192ca3615e443c10ec058497c2b6785c9c960940de15583930ad97c751aeffffffff01900e4d000000000017a914497d58d263bd82b460bdc9b8c69ca6e25acc99938700000000'))

>>> tx.vin[0].scriptSig
CScript([0,
x('304402207bf94f0f703364bbc008fe814061a36f421a59153b49131b828e583802cb13670220301095b2cb26623840455155e97ed1831d05a50798128a21ae83e38e8bcd906801'),
x('512102494948b6192ca3615e443c10ec058497c2b6785c9c960940de15583930ad97c751ae')])

>>> CScript(x('512102494948b6192ca3615e443c10ec058497c2b6785c9c960940de15583930ad97c751ae'))
CScript([
   1, x('02494948b6192ca3615e443c10ec058497c2b6785c9c960940de15583930ad97c7'), 1,
   OP_CHECKMULTISIG
])

Before #187 was merged, such script was wrongly detected as a witness program, which led to the assertion in SignatureHash.

There's nothing to make a pull request for. Well, except maybe add test case for CScript.is_witness_scriptpubkey, but I am not inclined to add this test, at least for now. It is a simple function and I am fairly certain that I copied the logic from Core fairly, and the logic now is the same as original. Core doesn't seem to have a specific test for CScript::IsWitnessProgram either.