Skip to content

Non-API breaking introduction of Schnorr keys#589

Merged
apoelstra merged 3 commits intorust-bitcoin:masterfrom
BP-WG:taproot/key-1
Apr 29, 2021
Merged

Non-API breaking introduction of Schnorr keys#589
apoelstra merged 3 commits intorust-bitcoin:masterfrom
BP-WG:taproot/key-1

Conversation

@dr-orlovsky
Copy link
Copy Markdown
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
Copy Markdown
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
Copy Markdown
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
Copy Markdown
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
Copy Markdown
Collaborator

utACK 230813b !

Copy link
Copy Markdown
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
@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

None yet

Development

Successfully merging this pull request may close these issues.

5 participants