keep-starknet-strange / shinigami

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

fix: fix opchecksig bug #223

Closed bloomingpeach closed 1 month ago

bloomingpeach commented 2 months ago

image

image

vercel[bot] commented 2 months ago

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

A member of the Team first needs to authorize it.

b-j-roberts commented 2 months ago

Hey, were you able to track down which line was giving the index out-of-bounds error? I'm curious where it was failing

bloomingpeach commented 2 months ago

it fails here: https://github.com/keep-starknet-strange/shinigami/blob/main/packages/engine/src/signature/signature.cairo#L298

By the way I think it is not supposed to go that far with an empty pubkey, https://github.com/keep-starknet-strange/shinigami/blob/main/packages/engine/src/signature/signature.cairo#L252 I think we should at least have a check for empty pubkey before or inside this function.

b-j-roberts commented 2 months ago

I see, thanks for the info! I think how you did it is a good solution.

However, I think we should also add a check here for if the pk_bytes.len != 65 and return an error, since uncompressed pub keys should always be of length 65 I think.

Would need to make parse_pub_key return an error too.

bloomingpeach commented 2 months ago

Would you like me to include the check in this PR or should it be in another PR? @b-j-roberts

b-j-roberts commented 2 months ago

Would you like me to include the check in this PR or should it be in another PR? @b-j-roberts

In this PR if you can, thanks

bloomingpeach commented 2 months ago

@b-j-roberts please review sir