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.

Sign up for GitHub

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

Jump to bottom

SharedSecret should have a "deserialize from bytes" method and serde impls #416

Closed
apoelstra opened this issue Mar 10, 2022 · 2 comments · Fixed by #417 or #418
Closed

SharedSecret should have a "deserialize from bytes" method and serde impls #416

apoelstra opened this issue Mar 10, 2022 · 2 comments · Fixed by #417 or #418

Comments

@apoelstra
Copy link
Member

See discussion in #402 -- it looks like we accidentally removed this functionality when un-genericizing the struct.
@dspicher dspicher mentioned this issue Mar 10, 2022
Merged
@tcharding tcharding mentioned this issue Mar 10, 2022
Merged
@tcharding
Copy link
Member

tcharding commented Mar 10, 2022 •
edited

For my sanity I went back looking for the serde impls and I didn't find them either in the un-genericizing work or in the previous genericizing work either. This begs the question, is anyone even using this SharedSecret?

@apoelstra
Copy link
Member Author

@tcharding I'm sure it has users, but by its nature it is an ephemeral thing which is not explicitly communicated, and I'm doubtful that anybody is trying to store/load it.

apoelstra added a commit that referenced this issue Mar 11, 2022
@apoelstra
Merge #418: Implement de/serialization for SharedSecret
49905b0 
de65fb2 Implement de/serialization for SharedSecret (Tobin Harding)

Pull request description:

  As we do for other keys implement serde de/serialization for the `SharedSecret`.

  Please note, this adds `from_slice` and `from_bytes` (borrowed and owner respectively) because I needed to use them. Doing so treads on @dspicher's toes because he is in the process of implementing an owned conversion method for `SharedSecret`. The fair thing to do would be let #417 resolve and merge before merging this one (I can rebase).

  ~Side note, its kind of rubbish that `BytesVisitor` deserializes into a buffer (presumably) then we reallocate and copy the buffer to use the borrowed conversion method due to the serde function signature `fn visit_bytes<E: de::Error>(self, v: &[u8]) -> Result<Self::Value, E>`~ (I was bumbling nonsense.)

  Closes: #416

ACKs for top commit:
  apoelstra:
    ACK de65fb2

Tree-SHA512: 3d484f160d8a459a867f645736713984bad982429236ac5351c20b6c21b42cec86e68009fe7adf062912037cf7e747e5b15357a5fd7900e52169f208a4e56710
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
2 participants
@apoelstra @tcharding