Skip to content

Feat/unified secp256k1 k256#7305

Open
rob-stacks wants to merge 16 commits into
stacks-network:developfrom
rob-stacks:feat/unified_secp256k1_k256
Open

Feat/unified secp256k1 k256#7305
rob-stacks wants to merge 16 commits into
stacks-network:developfrom
rob-stacks:feat/unified_secp256k1_k256

Conversation

@rob-stacks

@rob-stacks rob-stacks commented Jun 11, 2026

Copy link
Copy Markdown
Contributor

Description

This patch unifies the secp256k1 curve api to use the k256 crate (https://crates.io/crates/k256)

Additional unit tests have been added to ensure the whole api is covered

Applicable issues

  • fixes #

Additional info (benefits, drawbacks, caveats)

Checklist

  • Test coverage for new or modified code paths
  • For new Clarity features or consensus changes, add property tests (see docs/property-testing.md)
  • Changelog fragment(s) or "no changelog" label added (see changelog.d/README.md)
  • Required documentation changes (e.g., rpc/openapi.yaml for RPC endpoints, event-dispatcher.md for new events)
  • New clarity functions have corresponding PR in clarity-benchmarking repo

@rob-stacks
rob-stacks marked this pull request as ready for review June 15, 2026 13:51
@brice-stacks

Copy link
Copy Markdown
Contributor

I added @benjamin-stacks as a reviewer since he's just been making changes in here. I'll review after he does a pass. 🙏

@benjamin-stacks benjamin-stacks left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just did an initial pass and pointed out a few things that we don't have to implement because they're already provided by those crates.

Will have a deeper look once develop with 30ee31d is merged into this branch.

Comment thread stacks-common/src/util/secp256k1/native.rs
Comment thread stacks-common/src/util/secp256k1/native.rs
Comment thread stacks-common/src/util/secp256k1/native.rs
Comment on lines +264 to +265
/// `recover_to_pubkey`, which already accepts both low-S and high-S signatures
/// (low-S is enforced separately, e.g. in `verify`, where it matters for consensus).

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No, low-S should be enforced in recover_to_pubkey in most cases as well, that was the purpose of #7163 (and the follow-up #7337).

There's a bunch of test failures now because of this.

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