-
Notifications
You must be signed in to change notification settings - Fork 152
Derive comparison traits for all secret key types #485
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
Conversation
Nice! cc @sanket1729 I'd like you to concept ACK before merging this |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ACK 8e781ca
@apoelstra, I see that rust-secp implements |
Don't mind merging this. Should we wait until rust-bitcoin/rust-secp256k1#471 is resolved? |
Will merge after #488 |
@sanket1729 it looks like they appeared in rust-bitcoin/rust-secp256k1#364 where there is no discussion about them :/ maybe we forgot at that time about the pitfalls of One thing I do remember saying is that any comparisons of secret keys is likely to be leaky anyway. Like, if you put secret keys into a Another thing I do remember is that we were considering doing something crazy like, hashing the secret keys and then comparing by hash. But then somebody (kixunil?) pointed out that there are coherency requirements between |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ACK 8e781ca
The public key types (
DescriptorPublicKey
,DescriptorXKey
,SinglePubKey
, andDefiniteDescriptorKey
) all have comparison derivations.The underlying private key types (
bitcoin::PrivateKey
,Fingerprint
,DerivationPath
,DescriptorXKey
, andExtendedPrivKey
) all have comparison derivations.Let's catch 'em all.