keep-starknet-strange / shinigami

Bitcoin Script VM in Cairo
https://shinigamibtc.dev
MIT License
64 stars 59 forks source link

[bug] [bitcoin-core test] Script sig push only with OP_DATA_X #213

Closed b-j-roberts closed 2 months ago

b-j-roberts commented 2 months ago

Issue

The following bitcoin-core test is failing to give the correct result :

ScriptSig: '0x47 0x30440220781ba4f59a7b207a10db87628bc2168df4d59b844b397d2dbc9a5835fb2f2b7602206ed8fbcc1072fe2dfc5bb25909269e5dc42ffcae7ec2bc81d59692210ff30c2b01 0x41 0x0479be667ef9dcbbac55a06295ce870b07029bfcdb2dce28d959f2815b16f81798483ada7726a3c4655da4fbfc0e1108a8fd17b448a68554199c47d08ffb10d4b8 0x19 0x76a91491b24bf9f5288532960ac687abb035127b1d28a588ac', ScriptPubKey: 'HASH160 0x14 0x7f67f0521934a57d3039f77f9f32cf313f3ac74b EQUAL'

This code should be passing, but is giving a p2sh not pushonly error. Meaning the code incorrectly thinks there is a non push opcode executing in the script sig.

Replicate

To replicate the issue you can use the latest main branch with the following command:

scarb cairo-run '[[85638276393621973070440747833762038835109559404799393187910797674478057313,86909496337183863383784027065381492514903567749884832550003272325187200050,177363752697333587235310883696927306277866326243318851319289282961132958000,97524436451982317673892134732723748388322835445513126535154450177907057975,179136184789703560512717167442297186868911711012651864941955116552898752800,85638168247374406239538967717457551343685761391366490633594109526334989624,97510519539923601993761630086687932825421787774280843363255809817559839030,180606225255916961601221101003006194707792801503616537556620282464007775024,178791065229179536206708672940427288373183406912420293474234105050176698470,180896097882979158584260668067901388369312854335007770127238730398027888998,93989497426980796014993626649677710159089529955033602953281774031285543221],943219043,4,[127663847454887939587139309636239821037690053989400832659848678725247121252],353288535665911301610098168118721640775750166835370514396478341266096460,30,[],1345475400,4]'

This comment might help some with how to debug : https://github.com/keep-starknet-strange/shinigami/pull/192#issuecomment-2322981566

jsandinoDev commented 2 months ago

Hi @b-j-roberts can I work on this?

onlydustapp[bot] commented 2 months ago

Hey @jsandinoDev! Thanks for showing interest. We've created an application for you to contribute to shinigami. Go check it out on OnlyDust!

mubarak23 commented 2 months ago

@b-j-roberts let me figure this bug out

onlydustapp[bot] commented 2 months ago

Hey @mubarak23! Thanks for showing interest. We've created an application for you to contribute to shinigami. Go check it out on OnlyDust!

Iwueseiter commented 2 months ago

Hi @b-j-roberts can I work on this?

onlydustapp[bot] commented 2 months ago

Hey @Iwueseiter! Thanks for showing interest. We've created an application for you to contribute to shinigami. Go check it out on OnlyDust!

b-j-roberts commented 2 months ago

Hi @b-j-roberts can I work on this?

@jsandinoDev Yes, for sure, I can assign you! Thank you! Let me know if you have any questions

b-j-roberts commented 2 months ago

@jsandinoDev Any luck debugging this yet? Did you have any issues?

jsandinoDev commented 2 months ago

Hi @b-j-roberts Yes I have been able to debug.

I have found this and I have some questions

The problematic function is is_push_only in the src/engine.cairo file.

Specifically here:

Screenshot 2024-09-16 at 9 46 33 PM

I think that the problem is the position of that validation.

I also think that I found the solution, I will create the PR.