keep-starknet-strange / shinigami

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

wip: fix sig_der test #251

Closed bloomingpeach closed 1 week ago

bloomingpeach commented 1 week ago

currently I am aiming on fixing related issue to SIG_DER error, first thing I've done is update the existing scripts accordingly.

And also I create a new helper script for only run the SIG_DER tests : tests/run-sig-der-tests.sh, and the tests is on tests/sig_der_failing_tests.json

vercel[bot] commented 1 week 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 1 week ago

Hey, thanks. Seems there are a lot of SIG_DER tests this could potentially fix. I can take a deeper look soon, but I noticed a couple tests that were passing before are now giving SIG_DER error results.

Might be worth looking into.

  ScriptSig: '0 0' -- ScriptPubKey: '1 0x21 0x02865c40293a680cb9c020e7b1e106d8c1916d3cef99aa431a56d253e69256dac0 1 CHECKMULTISIG NOT' -- Flags: 'STRICTENC' -- Expected: OK
  Expected : OK -- Result   : SIG_DER
  ^[[0;31mFAIL^[[0m
scarb cairo-run --package shinigami_cmds '[[],3153968,3,[86797668792800902634759660184717057194939896983113578517425642626872259120,178833796868394003489311767857203710869216261298125424937602268936145286756,88709531411426821719016062832706955173577765959701167948755959603171631182],20308,2,[],1537155757518694469187,9]'
     Running shinigami_cmds
Running Bitcoin Script with ScriptSig: '0 0', ScriptPubKey: '1 0x21 0x02865c40293a680cb9c020e7b1e106d8c1916d3cef99aa431a56d253e69256dac0 1 CHECKMULTISIG NOT' and Flags: 'STRICTENC'
Execution failed: Signature DER error
Run completed successfully, returning [0]

  ScriptSig: '0 0x47 0x3044022044dc17b0887c161bb67ba9635bf758735bdde503e4b0a0987f587f14a4e1143d022009a215772d49a85dae40d8ca03955af26ad3978a0ff965faa12915e9586249a501 0x47 0x3044022044dc17b0887c161bb67ba9635bf758735bdde503e4b0a0987f587f14a4e1143d022009a215772d49a85dae40d8ca03955af26ad3978a0ff965faa12915e9586249a501' -- ScriptPubKey: '2 0 0x21 0x02865c40293a680cb9c020e7b1e106d8c1916d3cef99aa431a56d253e69256dac0 2 CHECKMULTISIG NOT' -- Flags: 'STRICTENC' -- Expected: OK
  Expected : OK -- Result   : SIG_DER
  ^[[0;31mFAIL^[[0m
scarb cairo-run --package shinigami_cmds '[[85030821728854912212103639134078053702964006258768454302692087137220899126,87254525100547216138737704407463574591211451173251962822336614354210926689,85203574242364798966473637736391103342906527145179793776979491736415188791,89033935833619398992168525023862704412162053266433865976743650390894524216,171718209788098576382437812964891619024125541979237380967585648309498312756,97398751181881103446764428367695131467343699862134368258622011906073966135,173822024168810095159582463443975364878490129286553452164396506234081523558,94030885787836686287116010850159398514782618971742482788832359674922940769,99311934435561283860364279238061322910324663746412015148896282079434008121],20281842709938480414733099515293384809826359958646833,22,[88564506589389073457714855955858846363583940029576408545492106665543689008,88676365850748824338134126615774255052137093683900969439013902277619573045,96101269732739450356016305886093115921589303891563216294708782741507492167],542003028,4,[],1537155757518694469187,9]'
     Running shinigami_cmds
Running Bitcoin Script with ScriptSig: '0 0x47 0x3044022044dc17b0887c161bb67ba9635bf758735bdde503e4b0a0987f587f14a4e1143d022009a215772d49a85dae40d8ca03955af26ad3978a0ff965faa12915e9586249a501 0x47 0x3044022044dc17b0887c161bb67ba9635bf758735bdde503e4b0a0987f587f14a4e1143d022009a215772d49a85dae40d8ca03955af26ad3978a0ff965faa12915e9586249a501', ScriptPubKey: '2 0 0x21 0x02865c40293a680cb9c020e7b1e106d8c1916d3cef99aa431a56d253e69256dac0 2 CHECKMULTISIG NOT' and Flags: 'STRICTENC'
Execution failed: Signature DER error
Run completed successfully, returning [0]
bloomingpeach commented 1 week ago

@b-j-roberts I am just confused the flag a little bit, do you think this is correct let strict_encoding = vm.has_flag(ScriptFlags::ScriptVerifyStrictEncoding) || vm.has_flag(ScriptFlags::ScriptVerifyDERSignatures);(in parse_base_sig_and_pk)

bloomingpeach commented 1 week ago

Also do you have an idea to prevent such regression in the future? @b-j-roberts, maybe a nice thing to have when working on fixing these remaining tests.

b-j-roberts commented 1 week ago

Also do you have an idea to prevent such regression in the future? @b-j-roberts, maybe a nice thing to have when working on fixing these remaining tests.

Hmm, not certain. One way could be to keep track of the passing tests with another run-passing-tests.sh script. It'll be super slow to run though. Could probably speed it up quite a bit by executing tests in parallel, but might be a little tricky to get right.