Skip to content

PSBT: Require compressed keys for BIP32 key fields#1100

Closed
dr-orlovsky wants to merge 1 commit intobitcoin:masterfrom
dr-orlovsky:patch-7
Closed

PSBT: Require compressed keys for BIP32 key fields#1100
dr-orlovsky wants to merge 1 commit intobitcoin:masterfrom
dr-orlovsky:patch-7

Conversation

@dr-orlovsky
Copy link
Copy Markdown
Contributor

BIP32 does not work with uncompressed public key; thus, all BIP32-related fields must explicitly require that the public key for the map key value must be serialized in compressed format

BIP32 does not work with uncompressed public key; thus, all BIP32-related fields must explicitly require that the public key for the map key value must be serialized in compressed format
@achow101
Copy link
Copy Markdown
Member

achow101 commented Apr 13, 2021

I'm actually not sure if that is strictly true. While xpubs are serialized with the compressed pubkey and BIP 32 uses the compressed pubkey in derivation, there is no specific mention of the compressed-ness of the derived key for actual use. Compressed keys are used as a convention, but I don't think they are required by BIP 32. Pubkeys can be easily un/compressed.

@dr-orlovsky
Copy link
Copy Markdown
Contributor Author

If we clearly see use of uncompressed keys as a bad practice, why we should not just update both standards specifying that non-compressed keys will be non-standard application for both BIP32 and PSBT?

@achow101
Copy link
Copy Markdown
Member

Note that there are some applications which use BIP 32 and produce uncompressed pubkeys. The one that comes to mind is Ledger devices.

@dr-orlovsky
Copy link
Copy Markdown
Contributor Author

They may continue doing so, but as a non-standard behavior. The idea of being "Standard" is about what devs will expect from API by default. It does not limit devs from what they can do in addition to that, but distinguishes recommended behavior / good practices.

sanket1729 added a commit to rust-bitcoin/rust-bitcoin 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
@murchandamus
Copy link
Copy Markdown
Member

@dr-orlovsky, @achow101: What’s the status on this pull request?

@achow101
Copy link
Copy Markdown
Member

It's been 3 years, so I'm going to go ahead and NACK this for the reasons previously stated.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants