vbuterin / pybitcointools

SImple, common-sense Bitcoin-themed Python ECC library
1.28k stars 857 forks source link

serialize_script / deserlialize_script fail round trip tests #104

Open brianddk opened 8 years ago

brianddk commented 8 years ago

Some round trip operations work, but others do not. Below is an example of a valid scriptPubKey that fails round-trip processing.

Python 2.7.9 (default, Dec 10 2014, 12:28:03) [MSC v.1500 64 bit (AMD64)] on win32
Type "help", "copyright", "credits" or "license" for more information.
>>> from bitcoin import *
>>> x = deserialize_script('76a914a89733100315c37d228a529853af341a9d290a4588ac')
>>> x
[118, 169, 'a89733100315c37d228a529853af341a9d290a45', 136, 172]
>>> y = serialize_script(x)
>>> y
'5b3131385d5b3136395d14a89733100315c37d228a529853af341a9d290a455b3133365d5b3137325d'
>>>
edmundedgar commented 8 years ago

Just to add, this survives the round trip as expected on the version that's still in Pip as pybitcointools (v1.1.15):

from pybitcointools import * x = deserialize_script('76a914a89733100315c37d228a529853af341a9d290a4588ac') x [118, 169, 'a89733100315c37d228a529853af341a9d290a45', 136, 172] y = serialize_script(x) y '76a914a89733100315c37d228a529853af341a9d290a4588ac'

edmundedgar commented 8 years ago

Probably related to #71, since it's not obvious that the workaround at #76 should be necessary.

wizardofozzie commented 8 years ago

See this PR: it's fixed.

serialize_script_unit was returning bytes([unit]), which should be chr(unit) for Python 2.7

edmundedgar commented 8 years ago

@simcity4242 Great stuff, the horse has already bolted but would it help anyone if I added a couple of tests for the cases you've just fixed?

wizardofozzie commented 8 years ago

@edmundedgar Most definitely. I've been re-writing some testing myself: test.py. I'm going to start writing a script.py for advanced scripting