keep-starknet-strange / shinigami

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

[bug] [bitcoin-core test] Script sig push only with NOP #215

Closed b-j-roberts closed 4 hours ago

b-j-roberts commented 6 days ago

Issue

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

ScriptSig: '0x47 0x3044022018a2a81a93add5cb5f5da76305718e4ea66045ec4888b28d84cb22fae7f4645b02201e6daa5ed5d2e4b2b2027cf7ffd43d8d9844dd49f74ef86899ec8e669dfd39aa01 NOP8 0x23 0x2103363d90d447b00c9c99ceac05b6262ee053441c7e55552ffe526bad8f83ff4640ac', ScriptPubKey: 'HASH160 0x14 0x215640c2f72f0d16b4eced26762035a42ffed39a EQUAL'

This code should be failing with a p2sh not pushonly error within the engine, but is passing. This is most likely due to the NOP8 opcode execution in the Script sig not giving the p2sh not pushonly error like it should.

Replicate

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

scarb cairo-run '[[85638276393621973070440747833762038827135161182803658775478804299459683170,94348312009021740139961859936069915624944684415242580173280269784951435832,177072616656797796472887515523897033852567714435353765129584358724158710837,177032522266426335070459730455281860734772707315710790496976700294919633254,97538213092435571229197708352016018136236349306739941081708643270112852000,85638222052479472070506766003547666308441769408253546983376385338131440485,172068737871228978941526372597074399983714305295778164763682290855266562614],510820426138955130342517808661815651,15,[127663847454887939587139309636239821030768701899862108006692212793516765494],677783736642793086246593772050010351370588991303528376563755713536803148,30,[],100686891288717405067299929,11]'

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

Kaminar-i commented 6 days ago

@b-j-roberts please can i work on this

onlydustapp[bot] commented 6 days ago

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

onlydustapp[bot] commented 6 days ago

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

PavitraAgarwal21 commented 6 days ago

Hey @b-j-roberts I want to work on this

onlydustapp[bot] commented 6 days ago

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

mubarak23 commented 6 days ago

This bug is happening due to the way PUSHONLY OPCODE was implemented

A quick log along the execution path should show what is happening

@b-j-roberts kindly assigned

b-j-roberts commented 5 days ago

@b-j-roberts please can i work on this

@Kaminar-i Yes, I can assign you. Welcome to the project. Please let me know if you have any questions, I will reach out in a couple days to check on progress 🚀

b-j-roberts commented 1 day ago

@Kaminar-i Any luck debugging this script yet? Did you have any questions?

jsandinoDev commented 1 day ago

Hi @Kaminar-i @b-j-roberts I think that the problem of this issue might be in the pull_data function, you could check it after #221 it is integrated

b-j-roberts commented 4 hours ago

This was resolved as part of this PR, thank you for looking into it.