Logic fixes for rounds with partial echo

[fjarri]

Fixes #21

Imagine the following round configuration (which is possible during KeyResharing from synedrion):

• node 0 sends an echo-broadcast to [1, 2, 3]
• node 1 sends an echo-broadcast to [2, 3]
• nodes 2 and 3 do not send echo-broadcasts

After we received the initial broadcasts, we want an echo round where the nodes 1, 2, 3 echo received messages to each other. That is:

• node 0 does not send or receive anything
• node 1 sends the broadcasts from [0] to 2 and 3, and expects [0, 1] from 2 and 3
• node 2 sends the broadcasts from [0, 1] to 1 and 3, and expects [0] from 1 and [0, 1] from 3
• node 3 sends the broadcasts from [0, 1] to 1 and 2, and expects [0] from 1 and [0, 1] from 2

(note that node 1 does not send its broadcast the second time; that would be pointless)

This requires adding another method to Round specifying the exact behavior of the node, since this information cannot be obtained from existing methods. This PR introduces Round::echo_round_participation() and EchoRoundParticipation enum.

In most cases we can use the blanket implementation returning EchoRoundParticipation, which means that the node either sends and receives echo messages, or does neither. This means that we can deduce the properties of the echo round from Round::message_destinations() and Round::expecting_messages_from(). The case where the node only sends echo messages (node 0), we don't need any additional info other than this fact. For the case where the node only receives echo messages (node 3), it needs additional info specifying which nodes actually participate in the echo round.

Edit: another possible API here is to get rid of EchoRoundParticipation enum and instead have a method returning Option<BTreeSet<Id>>. None would cover both Default and Send, and we will distinguish between them by checking if expecting_messages_from() is empty or not. Would that work?

[coveralls]

Pull Request Test Coverage Report for Build 11864171410

Details

• 59 of 59 (100.0%) changed or added relevant lines in 6 files are covered.
• No unchanged relevant lines lost coverage.
• Overall coverage increased (+0.8%) to 69.831%

---

Totals
Change from base Build 11864158435:	0.8%
Covered Lines:	1773
Relevant Lines:	2539

---

💛 - Coveralls

[dvdplm]

I'm struggling a bit to review this. The code seems all ok, modulo nitpicks, but I'm kind of stuck in my head on why we need this (despite the great PR description). E.g. in the partial_echo test, how should I think about a real world scenario there? Would node 0 be like a "dealer" of some sort, broadcasting but not participating? And what would node 4 be, given it's not participating at all?

[dvdplm]

Maybe expecting_echos_from for consistency with expecting_message_from?

[fjarri]

It would be a little ambiguous I think. expected_echos is the set of echos you're expecting to find in a pack that's sent to you (from any node), and expecting_echos_from could be understood as the nodes that need to send the echo packs to you. These can be different.

[dvdplm]

Hmm. Maybe just echoes works then?

[dvdplm]

Would Full be a good name here? It doesn't fully capture the semantics but is at least a hint in the right direction whereas "default" doesn't mean anything unless the reader knows what the default is.

[fjarri]

Funny you should say that, Full was the original name I had here :) But I changed it to Default because it also covers the nodes which don't send or receive anything (node 4 in the test), where I wasn't sure Full was intuitive.

[dvdplm]

Full isn't perfect, but gives a hint to the right understanding. I like it better than Default.

We could go with a FullOrAbsent but that reads very confusingly. :)

[fjarri]

but I'm kind of stuck in my head on why we need this (despite the great PR description).

This is the exact scenario of what's happening during KeyResharing. There is a set of nodes that sends broadcasts, and there is a set of nodes that receives broadcasts, and these sets are not equal (maybe not even intersecting). The ones that send broadcasts are the old holders of key shares, and the receivers are the new holders.

The assumption I'm making here is that only the "target" nodes will need to participate in the echo round, making sure they each received the same messages; the senders can do without.

E.g. in the partial_echo test, how should I think about a real world scenario there? Would node 0 be like a "dealer" of some sort, broadcasting but not participating? And what would node 4 be, given it's not participating at all?

The node 4 is just to cover all possibilities, that's something that doesn't happen during KeyResharing.

[dvdplm]

There's an argument to prefer tinyvec to smallvec, given the former has a "no unsafe" policy (and the latter has had a few memory related bugs over the years).

Another tangential thought: RustCrypto use the hybrid_array so we probably have it in the dependency tree. I haven't looked into it but perhaps it can be used here too?

[fjarri]

hybrid_array deals exclusively with stack arrays, I think.

[dvdplm]

It's too bad SmallVec isn't Copy. :/