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

Non-API breaking introduction of Schnorr keys #589

Merged
merged 3 commits into from Apr 29, 2021
Merged

Non-API breaking introduction of Schnorr keys #589

merged 3 commits into from Apr 29, 2021

Conversation

dr-orlovsky
Copy link
Collaborator

@dr-orlovsky dr-orlovsky commented Apr 12, 2021

First two steps of #588, which are non-API breaking and can get into 0.26.1:

  • Putting pre-Schnorr ECDSA keys under util::ecdsa, while still re-exporting them as util::key such that downstream dependencies will not break
  • Updating all internal references to the keys to util::ecdsa
  • Introducing util::schnorr mod re-exporting Secp256k1 Schnorr key types
  • Making use of util::key ECDSA keys deprecated

This is the first step in introducing Schnorr key support as per #588
This is second step in introducing Schnorr key support as per #588
Copy link
Contributor

@thomaseizinger thomaseizinger left a comment

Choose a reason for hiding this comment

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

ACK 230813b

Comment on lines +90 to +91
pub use util::ecdsa;
pub use util::schnorr;
Copy link
Contributor

Choose a reason for hiding this comment

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

👍

This is a tiny step towards #525 where the util module was made private.

Copy link
Member

@apoelstra apoelstra left a comment

Choose a reason for hiding this comment

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

ack 230813b

@stevenroose
Copy link
Collaborator

utACK 230813b !

Taproot automation moved this from Ready for Review to Ready for merge Apr 28, 2021
Copy link
Contributor

@sgeisler sgeisler left a comment

Choose a reason for hiding this comment

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

utACK 230813b

@apoelstra apoelstra merged commit 4db4e60 into rust-bitcoin:master Apr 29, 2021
Taproot automation moved this from Ready for merge to Done Apr 29, 2021
@dr-orlovsky dr-orlovsky mentioned this pull request May 14, 2021
20 tasks
sanket1729 added a commit that referenced this pull request Jan 9, 2022
… of bitcoin ECDSA

cf0c48c Improve Debug for PrivateKey (Dr Maxim Orlovsky)
b65a6ae Test for extended private key keypair generation  f5875a (Dr Maxim Orlovsky)
e6a3d60 BIP32 extended key `to_ecdsa()` and `to_schnorr()` methods (Dr Maxim Orlovsky)
b72f56c BIP32 extended keys are using Scep256k1 keys instead of bitcoin ECDSA (Dr Maxim Orlovsky)

Pull request description:

  This is third step required to introduce Schnorr key support according to #588. This PR starts API-breaking changes and is follow-up to non-API breaking #589, which is already merged.

  PR rationale: BIP32 does not support uncompressed keys and using type with compression flag was a mistake

ACKs for top commit:
  apoelstra:
    ACK cf0c48c
  sanket1729:
    ACK cf0c48c. #757 might need rework after this

Tree-SHA512: 6356a65004e7517256bacbf9aaeb69a22fd8536b341e567c5c4e819288e1105d083fe12ac0641404c407c97acf039bdc525f8e02b1b594a6cdda90106f3b1bdc
sanket1729 added a commit that referenced this pull request Jan 11, 2022
…n ECDSA

a6e8f58 PSBT BIP32 keys moved to Secp256k1 from bitcoin ECDSA (Dr Maxim Orlovsky)

Pull request description:

  Fourth step in implementation of Schnorr key support after #588. This PR is a follow-up to non-API breaking #589 and API-breaking #590, which must be reviewed and merged first. ~~(The current PR includes all commits from #589 and #590, which should be reviewed there. The only commit specific to this PR is b8105e9)~~

  UPDATE: All related PRs are merged now and this PR is ready for the review

  PR description:
  While PSBT BIP174 does not specify whether uncompressed keys are supported in BIP32-related fields, from BIP32 it follows that it is impossible to use uncompressed keys within the extended keys.  This PR fixes this situation and is a companion to BIP174 PR clarifying key serialization: bitcoin/bips#1100

ACKs for top commit:
  apoelstra:
    ACK a6e8f58
  sanket1729:
    ACK a6e8f58. Not sure which order to merge since there are many ready PRs which that would break each other.

Tree-SHA512: 198ba646bbce1949b255a54a97957d952acdad8b7f9580be123116c0f44d773e6d90e0cac0d5993ec9a6b3328aa43aced0908522817861585877c50008fec835
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
minor API Change This PR should get a minor version bump
Projects
Development

Successfully merging this pull request may close these issues.

None yet

5 participants