keep-starknet-strange / shinigami

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

[FEAT] implement p2pkh #197

Closed prasincs closed 1 month ago

prasincs commented 1 month ago

I'm trying to do a few naive things first to make sure I understand where the code is right now. I'm trying to replicate some existing tests and it seems to be failing on CHECKSIG and it's unclear if it's some test setup diff or I'm missing something because test_checksig_valid seems to work.

vercel[bot] commented 1 month ago

@prasincs is attempting to deploy a commit to the keep-starknet-strange Team on Vercel.

A member of the Team first needs to authorize it.

j1mbo64 commented 1 month ago

Hi @prasincs , I think it's because OP_CHECKSIG use the whole transaction to generate signature. You use mock_transaction which mock the transaction according to the signature in test_checksig_valid. You can try first to make a Transaction who match the desired. Maybe you can write down the txid you are trying to simulate ?

prasincs commented 1 month ago

Hi @prasincs , I think it's because OP_CHECKSIG use the whole transaction to generate signature. You use mock_transaction which mock the transaction according to the signature in test_checksig_valid. You can try first to make a Transaction who match the desired. Maybe you can write down the txid you are trying to simulate ?

I wasn't picking a specific transaction at the time, I was merely trying to pattern match some existing tests to get more familiar with the code. @b-j-roberts pointed me to the right direction. I'm now trying to replicate the first p2pkh tx https://learnmeabitcoin.com/explorer/tx/6f7cf9580f1c2dfb3c4d5d043cdbb128c640e3f20161245aa7372e9666168516#output-0 it's failing on verifying signature, which seems appropriate. Is this verification the missing part?

prasincs commented 1 month ago

I haven't had much time to look into this but I'd suspected something was wrong with signature validation. So I compared the values from the tx that spent the first P2PKH tx and the intermediate values for message and while the values from test_validate_transaction pass, what I have with the P2PKH doesn't. I've pushed it over here if you want to check and run for yourself https://gist.github.com/prasincs/3c133f3fc75931c1f6c4408a6231ae54

b-j-roberts commented 1 month ago

It seems the test was failing due to a bug mentioned and fixed here. https://github.com/keep-starknet-strange/shinigami/pull/209#discussion_r1762318707

Thank you for diving into this, was tricky to find the bug.