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

implement Deref trait for SecretKey #245

Closed
toxeus opened this issue Nov 3, 2020 · 8 comments
Closed

implement Deref trait for SecretKey #245

toxeus opened this issue Nov 3, 2020 · 8 comments

Comments

@toxeus
Copy link

toxeus commented Nov 3, 2020

The Deref trait is implemented for SharedSecret and SerializedSignature. I was wondering if you have considered implementing that trait also for PublicKey and SecretKey already?

@apoelstra
Copy link
Member

What would they Deref to? The internal representation is opaque and subject to change.

@toxeus
Copy link
Author

toxeus commented Nov 3, 2020

I thought they would Deref to &[u8].

@toxeus
Copy link
Author

toxeus commented Nov 3, 2020

SecretKey already allows access to the internal representation https://docs.rs/secp256k1/0.19.0/secp256k1/key/struct.SecretKey.html#method.as_ref. I think implementing Deref would be useful and consistent with the types mentioned above.

@elichai
Copy link
Member

elichai commented Nov 3, 2020

A. The internal representation of PublicKey is Implementation Defined, so to impl Deref you'd have to serialize it each time.
B. I actually think we should decrease things magic implementations like Deref, not increase, because they give a bunch of functions that don't really makes sense for SecretKey (on the other hand I do love abusing it sometimes because you magically get a bunch of functions for free)

@toxeus
Copy link
Author

toxeus commented Nov 4, 2020

I agree with A. Let me adjust the issue title to reflect that.

About B: Deref is widely used in Rust and part of the language. Personally, I like it. I can understand that some perceive it as "magic" but this can be argued about many language features really and if you strictly go down that path you end up with golang ;)

Still, the inconsistency between SharedSecret and SerializedSignature on one hand and SecretKey on the other still exists. I'd like to understand when "magic" is warranted and when not.

@toxeus toxeus changed the title implement Deref trait for PublicKey and SecretKey implement Deref trait for SecretKey Nov 4, 2020
@elichai
Copy link
Member

elichai commented Nov 4, 2020

About B: Deref is widely used in Rust and part of the language. Personally, I like it. I can understand that some perceive it as "magic" but this can be argued about many language features really and if you strictly go down that path you end up with golang

Don't get me started on "magic" in golang :) (comments are magic, the amount of return values from maps/channels is magically chosen by how many lvalues you have, cgo is 100% pure magic, which pointers the GC tracks is magic, should I continue? :) )

@apoelstra
Copy link
Member

I think Deref in Rust is uniquely magical :)

@apoelstra
Copy link
Member

In any case -- I think we should probably go in the direction of removing access to the internal representation (or providing a convert_to_ffi method in its stead). The internal representation is almost certainly not what the user wants unless they are doing magic FFI stuff (and then I would argue that they should never have started using the non-FFI types to begin with..). See #248 for example of where people got burned by the internal representation not resembling an actual public key.

@toxeus toxeus closed this as completed Feb 24, 2021
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

No branches or pull requests

3 participants