Implement serde traits for message types to support wasm_bindgen

[tmpfs]

Issue summary

Message types do not implement the serde Serialize and Deserialize traits which means we can't pass messages across the webassembly/javascript bindings.

I will look into adding serde support to ProtocolMessage etc so we can create bindings in webassembly.

Just wanted to let you know about this.

Other information and links

The multi-party-ecdsa library implements Serialize and Deserialize for all message types and LocalKey so that it works with wasm_bindgen so I think the CGGMP implementation should to.

Thanks!

[tmpfs]

@drewstone, looks like I will need to implement these traits for various types in fs-dkr as well so I can implement on ProtocolMessage - are you ok to merge/support this if I prepare some PRs?

[tmpfs]

So I noticed that they are implemented for JoinMessage and RefreshMessage in fs-dkr which is great. Maybe you can advise on the best way to approach this given these errors when I try to derive for M:

error[E0277]: the trait bound `Sha256: Serialize` is not satisfied
   --> src/refresh/state_machine.rs:320:9
    |
320 |     Round1(Option<JoinMessage<Secp256k1, Sha256, { crate::utilities::STAT_PARAM }>>),
    |            ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ the trait `Serialize` is not implemented for `Sha256`
    |
    = help: the following other types implement trait `Serialize`:
              &'a T
              &'a mut T
              ()
              (T0, T1)
              (T0, T1, T2)
              (T0, T1, T2, T3)
              (T0, T1, T2, T3, T4)
              (T0, T1, T2, T3, T4, T5)
            and 371 others
    = note: required because of the requirements on the impl of `Serialize` for `fs_dkr::add_party_message::JoinMessage<Secp256k1, Sha256, 80_usize>`
    = note: 1 redundant requirement hidden
    = note: required because of the requirements on the impl of `Serialize` for `std::option::Option<fs_dkr::add_party_message::JoinMessage<Secp256k1, Sha256, 80_usize>>`
note: required by a bound in `serialize_newtype_variant`
   --> /Users/muji/.cargo/registry/src/github.com-1ecc6299db9ec823/serde-1.0.147/src/ser/mod.rs:940:12
    |
940 |         T: Serialize;
    |            ^^^^^^^^^ required by this bound in `serialize_newtype_variant`

error[E0277]: the trait bound `fs_dkr::error::FsDkrError: Serialize` is not satisfied
   --> src/refresh/state_machine.rs:322:3
    |
322 |         Option<FsDkrResult<RefreshMessage<Secp256k1, Sha256, { crate::utilities::STAT_PARAM }>>>,
    |         ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ the trait `Serialize` is not implemented for `fs_dkr::error::FsDkrError`
    |
    = help: the following other types implement trait `Serialize`:
              &'a T
              &'a mut T
              ()
              (T0, T1)
              (T0, T1, T2)
              (T0, T1, T2, T3)
              (T0, T1, T2, T3, T4)
              (T0, T1, T2, T3, T4, T5)
            and 371 others
    = note: required because of the requirements on the impl of `Serialize` for `std::result::Result<fs_dkr::refresh_message::RefreshMessage<Secp256k1, Sha256, 80_usize>, fs_dkr::error::FsDkrError>`
    = note: 1 redundant requirement hidden
    = note: required because of the requirements on the impl of `Serialize` for `std::option::Option<std::result::Result<fs_dkr::refresh_message::RefreshMessage<Secp256k1, Sha256, 80_usize>, fs_dkr::error::FsDkrError>>`
note: required by a bound in `serialize_newtype_variant`
   --> /Users/muji/.cargo/registry/src/github.com-1ecc6299db9ec823/serde-1.0.147/src/ser/mod.rs:940:12
    |
940 |         T: Serialize;
    |            ^^^^^^^^^ required by this bound in `serialize_newtype_variant`

error[E0277]: the trait bound `Sha256: Serialize` is not satisfied
   --> src/refresh/state_machine.rs:322:3
    |
322 |         Option<FsDkrResult<RefreshMessage<Secp256k1, Sha256, { crate::utilities::STAT_PARAM }>>>,
    |         ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ the trait `Serialize` is not implemented for `Sha256`
    |
    = help: the following other types implement trait `Serialize`:
              &'a T
              &'a mut T
              ()
              (T0, T1)
              (T0, T1, T2)
              (T0, T1, T2, T3)
              (T0, T1, T2, T3, T4)
              (T0, T1, T2, T3, T4, T5)
            and 371 others
    = note: required because of the requirements on the impl of `Serialize` for `fs_dkr::refresh_message::RefreshMessage<Secp256k1, Sha256, 80_usize>`
    = note: 2 redundant requirements hidden
    = note: required because of the requirements on the impl of `Serialize` for `std::option::Option<std::result::Result<fs_dkr::refresh_message::RefreshMessage<Secp256k1, Sha256, 80_usize>, fs_dkr::error::FsDkrError>>`
note: required by a bound in `serialize_newtype_variant`
   --> /Users/muji/.cargo/registry/src/github.com-1ecc6299db9ec823/serde-1.0.147/src/ser/mod.rs:940:12
    |
940 |         T: Serialize;
    |            ^^^^^^^^^ required by this bound in `serialize_newtype_variant`

error[E0277]: the trait bound `Sha256: Deserialize<'_>` is not satisfied
    --> src/refresh/state_machine.rs:320:9
     |
320  |     Round1(Option<JoinMessage<Secp256k1, Sha256, { crate::utilities::STAT_PARAM }>>),
     |            ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ the trait `Deserialize<'_>` is not implemented for `Sha256`
     |
     = help: the following other types implement trait `Deserialize<'de>`:
               &'a Path
               &'a [u8]
               &'a serde_bytes::bytes::Bytes
               &'a str
               ()
               (T0, T1)
               (T0, T1, T2)
               (T0, T1, T2, T3)
             and 603 others
     = note: required because of the requirements on the impl of `Deserialize<'_>` for `fs_dkr::add_party_message::JoinMessage<Secp256k1, Sha256, 80_usize>`
     = note: 1 redundant requirement hidden
     = note: required because of the requirements on the impl of `Deserialize<'_>` for `std::option::Option<fs_dkr::add_party_message::JoinMessage<Secp256k1, Sha256, 80_usize>>`
note: required by a bound in `newtype_variant`
    --> /Users/muji/.cargo/registry/src/github.com-1ecc6299db9ec823/serde-1.0.147/src/de/mod.rs:2123:12
     |
2123 |         T: Deserialize<'de>,
     |            ^^^^^^^^^^^^^^^^ required by this bound in `newtype_variant`

error[E0277]: the trait bound `fs_dkr::error::FsDkrError: Deserialize<'_>` is not satisfied
    --> src/refresh/state_machine.rs:322:3
     |
322  |         Option<FsDkrResult<RefreshMessage<Secp256k1, Sha256, { crate::utilities::STAT_PARAM }>>>,
     |         ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ the trait `Deserialize<'_>` is not implemented for `fs_dkr::error::FsDkrError`
     |
     = help: the following other types implement trait `Deserialize<'de>`:
               &'a Path
               &'a [u8]
               &'a serde_bytes::bytes::Bytes
               &'a str
               ()
               (T0, T1)
               (T0, T1, T2)
               (T0, T1, T2, T3)
             and 603 others
     = note: required because of the requirements on the impl of `Deserialize<'_>` for `std::result::Result<fs_dkr::refresh_message::RefreshMessage<Secp256k1, Sha256, 80_usize>, fs_dkr::error::FsDkrError>`
     = note: 1 redundant requirement hidden
     = note: required because of the requirements on the impl of `Deserialize<'_>` for `std::option::Option<std::result::Result<fs_dkr::refresh_message::RefreshMessage<Secp256k1, Sha256, 80_usize>, fs_dkr::error::FsDkrError>>`
note: required by a bound in `newtype_variant`
    --> /Users/muji/.cargo/registry/src/github.com-1ecc6299db9ec823/serde-1.0.147/src/de/mod.rs:2123:12
     |
2123 |         T: Deserialize<'de>,
     |            ^^^^^^^^^^^^^^^^ required by this bound in `newtype_variant`

error[E0277]: the trait bound `Sha256: Deserialize<'_>` is not satisfied
    --> src/refresh/state_machine.rs:322:3
     |
322  |         Option<FsDkrResult<RefreshMessage<Secp256k1, Sha256, { crate::utilities::STAT_PARAM }>>>,
     |         ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ the trait `Deserialize<'_>` is not implemented for `Sha256`
     |
     = help: the following other types implement trait `Deserialize<'de>`:
               &'a Path
               &'a [u8]
               &'a serde_bytes::bytes::Bytes
               &'a str
               ()
               (T0, T1)
               (T0, T1, T2)
               (T0, T1, T2, T3)
             and 603 others
     = note: required because of the requirements on the impl of `Deserialize<'_>` for `fs_dkr::refresh_message::RefreshMessage<Secp256k1, Sha256, 80_usize>`
     = note: 2 redundant requirements hidden
     = note: required because of the requirements on the impl of `Deserialize<'_>` for `std::option::Option<std::result::Result<fs_dkr::refresh_message::RefreshMessage<Secp256k1, Sha256, 80_usize>, fs_dkr::error::FsDkrError>>`
note: required by a bound in `newtype_variant`
    --> /Users/muji/.cargo/registry/src/github.com-1ecc6299db9ec823/serde-1.0.147/src/de/mod.rs:2123:12
     |
2123 |         T: Deserialize<'de>,
     |            ^^^^^^^^^^^^^^^^ required by this bound in `newtype_variant`

For more information about this error, try `rustc --explain E0277`.

The main issues seem to be the Sha256 type and the use of FsDkrResult, I would have thought that Round2 should wrap RefreshMessage rather than FsDkrResult - is that feasible?

[tmpfs]

Ok, so removing FsDkrResult was pretty straightforward and the tests continue to pass (for now I just want to focus on the refresh module): https://github.com/webb-tools/cggmp-threshold-ecdsa/pull/17/commits/cc72049a370afd010565663e283b7f3096a75798 - I think it makes the code cleaner too 👍

[tmpfs]

~I think to workaround the Sha256 problem I can't derive Serialize and Deserialize but will need to implement them manually on M.~

Nope, we need a type that implements Digest + Clone + Serialize + Deserialize and use that in place of the Sha256 type - the only thing I can think of is a new type struct that wraps an inner sha2::Sha256 and proxies the Digest implementation.

What do you think?

[tmpfs]

So the only way I think this is possible is with a new type that wraps sha2::Sha256, I added it here: https://github.com/webb-tools/cggmp-threshold-ecdsa/pull/17/commits/04e695f3e94f7c21677029e9841f75b7a3601569

And it compiles and all the tests pass.

Will implement for the sign and presign modules and then I think this is ready for review.

[tmpfs]

Cool, derived the serde traits for the sign and presign modules - a review would be appreciated: https://github.com/webb-tools/cggmp-threshold-ecdsa/pull/17

Thanks 🙏