axelarnetwork / tofn

A threshold cryptography library in Rust
Apache License 2.0
107 stars 22 forks source link

Automatically validate deserialized structs #177

Open ggutoski opened 2 years ago

ggutoski commented 2 years ago

Serde does not automatically validate deserialized structs. This is annoying because a struct instance that is otherwise guaranteed to be valid by construction can be made invalid via serde round trip. Examples in tofn include GroupPublicInfo and SharePublicInfo.

I spent a long time digging for ergonomic ways to validate serde-deserialized data. :disappointed: Seems like the best solution is to use deserialize_with as described here https://github.com/serde-rs/serde/pull/1858#pullrequestreview-575089757. Documentation on deserialize_with is sparse---this is all I could find: Field attributes · Serde

If we do go this route then presumably the clean way to do it is for each struct would have a validate() method that is called in two places:

  1. at initial construction (eg. in new())
  2. in deserialize_with

That's a pretty generic pattern---might even be worth a Validator trait or something. Note that this crate exists https://github.com/Keats/validator but at first glance it doesn't seem to meet our needs.

Not sure yet whether we have a strong need for this fix but I can envision a time when we do.

tarcieri commented 2 years ago

A pattern I'd recommend for this is having a separate "serde type" where you derive Serialize/Deserialize, and then impl TryFrom to convert from the "serde type" to a validated struct. You can then have deserialize_with decode to the "serde type" and then call TryFrom

ggutoski commented 2 years ago

A pattern I'd recommend for this is having a separate "serde type" ...

Thanks for the suggestion! This pattern is described in detail at https://github.com/serde-rs/serde/issues/642#issuecomment-683276351

tarcieri commented 2 years ago

TIL:

#[serde(try_from = "MyTypeShadow")]