Skip to content
This repository has been archived by the owner on Mar 11, 2024. It is now read-only.

Update ecdsa_secp256k1.nr with a test #12

Merged
merged 3 commits into from
Oct 27, 2023
Merged

Conversation

skaunov
Copy link
Contributor

@skaunov skaunov commented Oct 24, 2023

Greetings!

I feel like this test should pass, but it fails! Could you, pls, indicate the cause of failure? Maybe I just got something wrong. =(

@shuklaayush
Copy link
Owner

The points are represented as jacobian coordinates and not usual projective coordinates.

You can check that it works if you change it to this

    let g_projective_z_doubled = Point{
        x: curve.gen.x.mul(two).mul(two),
        y: curve.gen.y.mul(two).mul(two).mul(two),
        z: two,
    };

@skaunov
Copy link
Contributor Author

skaunov commented Oct 26, 2023

Thank you for the link and sorry for bothering!

Should I just close the PR or is it better to add a corrected version of the test that you posted so that future reader won't fall into the same misunderstanding?

@shuklaayush
Copy link
Owner

We can add a test. Can you fix it?

@skaunov
Copy link
Contributor Author

skaunov commented Oct 26, 2023

yeah, of course
I'll update this one!

@shuklaayush shuklaayush merged commit 30e95e1 into shuklaayush:main Oct 27, 2023
1 check failed
@skaunov skaunov deleted the patch-1 branch November 28, 2023 13:42
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants