Add serialize function for schnorr::Signature #607
Conversation
isaac-asdf
changed the title
|
I'm a bit curious what the use case for this is -- we already have |
|
Ah, that worked and got me what I needed. I was struggling with figuring out how to get the bytes for the underlying data but the |
|
I think this would still be nice, especially if we add |
+1 . I would still find having this useful. |
|
Would y'all like me to reopen then and update it as |
|
@isaac-asdf, yes. Unless @apoelstra has some reasons against it. As you mentioned, it is not very intuitive to find how to covert schnorr sig into bytes. |
|
Please remember to change the commit description too. (I usually don't ask to fix commit messages, I ask here because it'd be confusing to keep the same.) |
isaac-asdf
changed the title
src/schnorr.rs
Outdated
| @@ -86,6 +86,9 @@ impl Signature { | |||
| } | |||
| } | |||
|
|
|||
| /// Returns a signature as a byte array. | |||
| pub fn serialize<T: AsRef<[u8]>>(self) -> [u8; constants::SCHNORR_SIGNATURE_SIZE] { self.0 } | |||
There was a problem hiding this comment.
@Kixunil is this what you meant by adding the AsRef? I haven't worked as much with generics yet so want to make sure I got that right
There was a problem hiding this comment.
No, I meant that in the future we may add impl AsRef<[u8]> for Signature and if we do the code using .as_ref may become ambiguous. The method helps disambiguate.
So this whole generic is not needed (surprised compiler didn't complain), just delete the entire generic.
Also stylistically I think &self is better since that's what most serialize methods are taking. But self is not strictly wrong.
And it occurred to me that you could put #[inline] on top of the method.
|
Can you squash the commits please. I'm pretty sure you want |
|
@tcharding the code is wrong for a different reason but note that |
isaac-asdf
force-pushed
the
add-schnorr-sig-to-bytes
branch
from
1a3836b
to
8af2cf1
Compare
convert from Signature to a byte_array