entropyxyz / synedrion

Implementation of Canetti-Gennaro-Goldfeder-Makriyannis-Peled threshold signing scheme
https://docs.rs/synedrion
GNU Affero General Public License v3.0
63 stars 10 forks source link

Access to MessageType (and SignedMessage?) #118

Closed tmpfs closed 3 weeks ago

tmpfs commented 6 months ago

For usage in real-world scenarios it's necessary to determine whether a message is broadcast or direct to route correctly but CombinedMessage does not expose this information as SignedMessage is private so I cannot call SignedMessage::message_type().

I can't think of a good reason why it needs to be private, are you ok to expose this?

Will submit a PR.

fjarri commented 6 months ago

The current approach is that all messages are considered to be direct ones to simplify the calling code. What platform are you running on that gives you access to broadcasting?

tmpfs commented 6 months ago

Hi @fjarri ,

I am using an E2EE relay server (tunnels messages over the noise protocol) but is able to broadcast and send directly. It would really help me if I could call message_type() to be able to route messages appropriately.

For some context, my use case is an MPC wallet rather than a blockchain which already has a network transport.

fjarri commented 6 months ago

It is not quite as simple as that. Currently Session::make_message() takes a destination, so simply exposing the message type will just make the API weird, because the whole structure of the public methods assumes direct messages only. I think a better approach is to have a public Session method to tell you if the current round's message is a pure broadcast, and then another method to request it (possibly the same method returning an Option). Then you will have to remember not to call make_message() in that round to avoid duplicates. I can't immediately think of a way to prevent that on the type level.

I wonder though, how critical it is to you to use broadcasts instead of direct messages when possible? Have you done any benchmarking? Not to say it's a bad idea, I just have absolutely no intuition on how big the benefits of it are. (In case you're worried that make_message() does extra work when there's only a broadcast - it doesn't, the broadcast, if any, is prepared when the round is started and then just copied)

tmpfs commented 6 months ago

Oh, I must be thinking about this wrong then. I am upgrading from a GG20 implementation which only yielded a single broadcast message to be dispatched to all other participants. But IIUC Session::message_destinations will always yield n-1 (or t-1 for the threshold protocols) messages regardless of whether its a direct, broadcast or combination round, is that correct?

If so, I will just route everything over the direct channels and ignore the broadcast channel on the network transport.

fjarri commented 6 months ago

But IIUC Session::message_destinations will always yield n-1 (or t-1 for the threshold protocols) messages regardless of whether its a direct, broadcast or combination round, is that correct?

That's right (in KeyResharing case, which is a little special, it will be n or n-1 messages, where n is the number of new share holders, -1 if the node is both an old and a new share holder).

I always thought GG'21 is not that different from GG'20 in terms of what messages it sends (the main changes being in the ZK proofs), so I'm surprised to hear GG'20 only sends broadcasts.

In GG'21, depending on the protocol, you may need to send a broadcast, n-1 direct messages, or both (e.g. KeyGen sends a broadcast in every round, KeyRefresh sends a broadcast in the first two rounds and a direct message in the 3rd, and Presigning sends both a broadcast and a direct message in the first round). In the current implementation this is generalized to always send a direct message (and if it's both a broadcast and a direct one, they're bundled into one message) because entropy-core does not have any means to send broadcasts.

If there's a benefit from sending a broadcast when it's possible, it can be made available as I described above, and it's not a bad idea, I'm just not sure it will be very noticeable in the overall performance (but I may be wrong here). So I think you can just use the existing API for direct messages for now, and I will leave this issue open for the future.

tmpfs commented 6 months ago

I always thought GG'21 is not that different from GG'20 in terms of what messages it sends (the main changes being in the ZK proofs), so I'm surprised to hear GG'20 only sends broadcasts.

Ah no, I must have phrased that badly, I meant that the underlying implementation only yielded a single broadcast message for broadcast rounds so I built into the network transport a concept of broadcasting (which effectively ends up being direct messages anyhow after resolving the public keys of the other participants and cloning the broadcast message). My understanding, is that the main difference (apart from the ZK proofs) between the GG20 and GG21 is the combined broadcast and direct round. A GG20 implementation would not need the CombinedMessage::Both variant.

Appreciate all the information and your work on this library, I think this is by far the best implementation I have come across (especially with the first-class webassembly support which is particularly important for our use case) - thank you 🙏

fjarri commented 3 weeks ago

All messages are bundled, so I don't think exposing this information is useful. But if you think further discussion is justified, please open an issue in https://github.com/entropyxyz/manul/issues