Skip to content

fix(util-crypto): use correct x/y getters in secp256k1Expand#2018

Merged
TarikGul merged 1 commit intopolkadot-js:masterfrom
AndreasGassmann:fix/secp256k1-expand-coords
Mar 13, 2026
Merged

fix(util-crypto): use correct x/y getters in secp256k1Expand#2018
TarikGul merged 1 commit intopolkadot-js:masterfrom
AndreasGassmann:fix/secp256k1-expand-coords

Conversation

@AndreasGassmann
Copy link
Copy Markdown
Contributor

@AndreasGassmann AndreasGassmann commented Jan 14, 2026

Replace px/py with x/y getters from @noble/curves ProjectivePoint. In recent versions of @noble/curves (1.4+), the px/py getters are buggy and return incorrect values. In 2.0, px/py were removed entirely (renamed to X/Y/Z). The x/y coordinate getters are the correct ones to use.

@TarikGul
Copy link
Copy Markdown
Member

TarikGul commented Mar 6, 2026

It needs a yarn lint

@TarikGul
Copy link
Copy Markdown
Member

TarikGul commented Mar 6, 2026

If I am understanding this correctly, we don't need to bump the version of @noble/curves because the x/y coords are still available, its just that for 1.4 px/py are buggy?

i'll look into this myself as well

@AndreasGassmann
Copy link
Copy Markdown
Contributor Author

AndreasGassmann commented Mar 10, 2026 •
edited
Loading

It needs a yarn lint

I tried running yarn lint, but there are many errors that seem unrelated to my change.

If I am understanding this correctly, we don't need to bump the version of @noble/curves because the x/y coords are still available, its just that for 1.4 px/py are buggy?

You do not need to bump the dependency version if this fix is merged. The bug exists from version 1.4 onwards, but with 2.0 this old, buggy, API was removed. This PR switches to the new, correct, API, which works on all versions (pre 1.4, 1.4+ and 2.0+).

@TarikGul TarikGul mentioned this pull request Mar 12, 2026
Closed
@TarikGul
Copy link
Copy Markdown
Member

TarikGul commented Mar 12, 2026

Okay I fixed the linter, you just need to pull in or rebase from master! Thanks again for the patience.

@AndreasGassmann AndreasGassmann force-pushed the fix/secp256k1-expand-coords branch from 2388758 to c0192c8 Compare March 12, 2026 17:05
@TarikGul TarikGul merged commit 372d460 into polkadot-js:master Mar 13, 2026
4 checks passed
@polkadot-js-bot
Copy link
Copy Markdown

polkadot-js-bot commented Mar 15, 2026

This pull request has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs.

@polkadot-js polkadot-js locked as resolved and limited conversation to collaborators Mar 15, 2026
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Reviewers

@TarikGul TarikGul TarikGul approved these changes

+1 more reviewer

@naserjr23 naserjr23 naserjr23 approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

5 participants

@AndreasGassmann @TarikGul @polkadot-js-bot @naserjr23 @rockbmb