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.

Sign up for GitHub

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

Jump to bottom

secp256k1_pedersen_commitment_serialize decompresses the point then recompresses it #293

Closed
RCasatta opened this issue May 10, 2024 · 0 comments · Fixed by #294
Closed

secp256k1_pedersen_commitment_serialize decompresses the point then recompresses it #293

RCasatta opened this issue May 10, 2024 · 0 comments · Fixed by #294

Comments

@RCasatta
Copy link 

          Ok, yeah, the issue is in libsecp256k1-zkp, not here. For some bizarre reason `secp256k1_pedersen_commitment_serialize` decompresses the point then recompresses it, when all it needs to do is a memcmp (I think). So this function is wasting a lot of time for no reason.

Originally posted by @apoelstra in ElementsProject/rust-elements#202 (comment)
apoelstra added a commit to apoelstra/secp256k1-zkp that referenced this issue May 10, 2024
@apoelstra
generator: massively speed up serialization
bafe257 
`secp256k1_pedersen_commit_serialize` would call `_load` (which does a
sqrt to fully decompress the key, then a conditional negation based on
the flag), then check the Jacobian symbol of the resulting y-coordinate,
then re-serialize based on this.

Instead, don't do any of this stuff. Copy the flag directly out of the
internal representation and copy the x-coordinate directly out of the
internal representation.

Checked that none of the other _serialize methods in the modules do
this.

Fixes BlockstreamResearch#293
@apoelstra apoelstra mentioned this issue May 10, 2024
Merged
apoelstra added a commit to apoelstra/secp256k1-zkp that referenced this issue May 16, 2024
@apoelstra
generator: massively speed up serialization
820fc6a 
`secp256k1_pedersen_commit_serialize` would call `_load` (which does a
sqrt to fully decompress the key, then a conditional negation based on
the flag), then check the Jacobian symbol of the resulting y-coordinate,
then re-serialize based on this.

Instead, don't do any of this stuff. Copy the flag directly out of the
internal representation and copy the x-coordinate directly out of the
internal representation.

Checked that none of the other _serialize methods in the modules do
this.

Fixes BlockstreamResearch#293
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

generator: massively speed up serialization apoelstra/secp256k1-zkp
1 participant
@RCasatta