Closed tmpfs closed 1 year ago
@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?
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?
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 👍
~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?
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.
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 🙏
Issue summary
Message types do not implement the serde
Serialize
andDeserialize
traits which means we can't pass messages across the webassembly/javascript bindings.I will look into adding
serde
support toProtocolMessage
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 implementsSerialize
andDeserialize
for all message types andLocalKey
so that it works withwasm_bindgen
so I think the CGGMP implementation should to.Thanks!