Bit-Wasp / bitcoin-lib-php

PHP libraries implementing bitcoin key functions, as well as BIP32 and electrum.
The Unlicense
136 stars 86 forks source link

Fix script parsing2 #85

Closed rubensayshi closed 9 years ago

rubensayshi commented 9 years ago

depends on #84

this is another OP_RETURN, but this time the rest of the script is not valid a valid sequence of pushdata-x + data, the last pushdata specifies more bytes than are available to read.

this is what bitcoin core gives as ASM: OP_RETURN 1127494704 30443133414435423941374541433936303141304531444236383543394335373445373143383046303032383737333633463936463736333046323231363937364136453346 444545334435423243443944353145343743364234344631363435353537454344314238384443344445374339333741393334364246434432 35394634433233393943353834424438383636393231333836384343433741373141453231354645324442434346324631433242 3441323642353231304141353731333836413437433543323134314444343244434230394641453844373432393430384442364631434139 [error]

this is what bitcore gives out as ASM (a bit easier to read): OP_RETURN 4 0x30343443 70 0x30443133414435423941374541433936303141304531444236383543394335373445373143383046303032383737333633463936463736333046323231363937364136453346 57 0x444545334435423243443944353145343743364234344631363435353537454344314238384443344445374339333741393334364246434432 52 0x35394634433233393943353834424438383636393231333836384343433741373141453231354645324442434346324631433242 56 0x3441323642353231304141353731333836413437433543323134314444343244434230394641453844373432393430384442364631434139 68 0x30373734304533433343303945324437

From IRC:

<Luke-Jr> rubensayshi: the last pushdata is invalid
<Luke-Jr> OP_RETURN, push-4-bytes, push-46-bytes, push-46-bytes, push-39-bytes, push-34-bytes, push-38-bytes, push-44-bytes
<Luke-Jr> but the last one is cut off before 44 bytes of data
<Luke-Jr> rubensayshi: the invalid opcode is never reached in the interpreter
Luke-Jr> rubensayshi: OP_RETURN terminates the script immediately. Furthermore, pubkey scripts are never executed until they are spent.
<Luke-Jr> So an unspent pubkey script can be basically anything (IIRC)
<rubensayshi> so the OP_RETURN can be (probably is) follow by some data that is not at all a valid script, the fact that it looks like "push-4-bytes, push-46-bytes, push-46-bytes, etc" is probably a coincidence?
<Luke-Jr> you could probably omit the OP_RETURN too (but then nodes couldn't prove it was unspendable/prunable

So I think I understand a little better now, and I guess giving NULL for the ASM in this case is fine ... since the ASM is just a nice human readable respresentation, however we can't make anything sensible out of this one ...

I feel a little stupid that up until now I've always sorta known that OP_RETURN terminates the script and now that I tried to dig a little deeper I started to confuse myself :/

rubensayshi commented 9 years ago

I guess we also don't support the OP_PUSHDATA_X opcodes, gotta work harder on bitcoin-php so I can move over :D

rubensayshi commented 9 years ago

I think the most useful thing to do for the ASM would be to just push all remaining data onto the stack after an OP_RETURN ... not sure if that's 'sane' though

afk11 commented 9 years ago

Yeah not fully yet, should add something for parsing pushdatas properly :s It's not often that you'll find something that tests that, but it could still happen.. Especially on testnet :)

afk11 commented 9 years ago

Think returning NULL for asm is fine - it's something that isn't really standardized, and in cases like this it will hardly be relied on..