Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix signature error message #1361

Merged
merged 2 commits into from
Aug 4, 2023

Conversation

FabijanC
Copy link
Contributor

@FabijanC FabijanC commented Aug 4, 2023

Description

In Pythonic cairo-vm, invalid signature was reported like this:

Error at pc=0:124:
Signature (1193313242338130160318297312666865822293235647311047835298033290753491994440, 155691799158656286221600221734062249357606752541106292108852403284952061129),
is invalid, with respect to the public key 3571077580641057962019375980836964323430604474979724507958294224671833227961,
and the message hash 1308364852790706196451346173687342589479412849559424395391024201449978019648.
Cairo traceback (most recent call last):
Unknown location (pc=0:577)
Unknown location (pc=0:541)
Unknown location (pc=0:245)

And in Rust cairo-vm, it's reported like this:

Error at pc=0:124:
Signature 03e4c994748ab4c6a0374e7aa6ae938b6c880a58ff2938ddfdd569bc694351990128cf025da10fcc6742f5c242922fb251c0852c7f68c70e1923fcc74f9454fb,
is invalid, with respect to the public key 2554664058264077495144519976141551974382939217789466639725771495363739387604,
and the message hash 3232999915494270110450164060897339602579794848593724915613880343018894519633.
Cairo traceback (most recent call last):
Unknown location (pc=0:615)
Unknown location (pc=0:600)
Unknown location (pc=0:245)

Notice how the signature is currently two concatenated hexadecimal representations (difficult to separate manually/visually), whereas it used to be two separated decimal representations (consistent with public key and message hash). This PR aims to address this.

I removed the checklist because I don't think it's needed for this small PR.

@codecov
Copy link

codecov bot commented Aug 4, 2023

Codecov Report

Merging #1361 (511b26e) into main (eab16c8) will not change coverage.
The diff coverage is 0.00%.

@@           Coverage Diff           @@
##             main    #1361   +/-   ##
=======================================
  Coverage   97.49%   97.49%           
=======================================
  Files          93       93           
  Lines       37854    37854           
=======================================
  Hits        36904    36904           
  Misses        950      950           
Files Changed Coverage Δ
vm/src/vm/runners/builtin_runner/signature.rs 96.73% <0.00%> (ø)

📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more

@pefontana pefontana added this pull request to the merge queue Aug 4, 2023
Merged via the queue into lambdaclass:main with commit 2191b5f Aug 4, 2023
36 of 38 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants