entropyxyz / manul

Round-based distributed protocols
https://docs.rs/manul
GNU Affero General Public License v3.0
6 stars 1 forks source link

Protocol combinators #60

Closed fjarri closed 1 week ago

fjarri commented 3 weeks ago

We have several different situations where we need to build on top of an existing protocol or combine several of them:

  1. Modifying messages the node sends, keeping Round::Protocol the same. This is useful for writing tests.
  2. Chaining two protocols together. This is useful e.g. for joining Presigning and Signing in CGGMP.
  3. Executing two independent protocols simultaneously. See #61, will be implemented in another PR.
  4. Building on top of an existing protocol, adding features. See #62, will be implemented in another PR.

Also:

Notes:

coveralls commented 3 weeks ago

Pull Request Test Coverage Report for Build 11823590883

Details


Changes Missing Coverage Covered Lines Changed/Added Lines %
manul/src/session/session.rs 24 26 92.31%
manul/src/session/evidence.rs 8 16 50.0%
manul/src/protocol/object_safe.rs 38 48 79.17%
manul/src/combinators/misbehave.rs 143 156 91.67%
manul/src/protocol/errors.rs 0 22 0.0%
manul/src/protocol/round.rs 17 53 32.08%
manul/src/combinators/chain.rs 159 280 56.79%
<!-- Total: 403 615 65.53% -->
Files with Coverage Reduction New Missed Lines %
manul/src/protocol/object_safe.rs 7 78.1%
<!-- Total: 7 -->
Totals Coverage Status
Change from base Build 11602146783: -2.5%
Covered Lines: 1731
Relevant Lines: 2509

💛 - Coveralls
dvdplm commented 1 week ago
  • Naming is kind of awkward. Suggestions?

I'm struggling with this. I don't love EntryPoint but it's pretty clear what it is/does. Let's go with that and if either of us have a brilliant idea we'll revisit.

  • Random idea: should EntryPoint::new take &self? This way we can basically formalize the concept of protocol constructors used in synedrion.

I like this!

  • Should misbehave go into testing? Or gated behind the testing feature?

I like misbehave to be difficult to misuse. Today we want to use it in tests, so let's put it there until we need it. I can totally envision a scenario where the core team wants to build a malicious node to test on (private?) testnets so maybe it'd be useful to have misbehave under a feature, but let's do that when the need arises.

  • The system with From bounds in the chain combinator is a little awkward, since we have to clone stuff for the first protocol. Some kind of a "splitting" trait might be more idiomatic, to split the full inputs into the inputs for Protocol 1 and the inputs for Protocol 2.

  • Also it requires us to impl From for the type from the original protocol, which may be foreign in practice.

Hmm. I like the idea of a "splitting" trait, but I don't have a strong opinion on this.

fjarri commented 1 week ago

Random idea: should EntryPoint::new take &self? This way we can basically formalize the concept of protocol constructors used in synedrion. Hmm. I like the idea of a "splitting" trait, but I don't have a strong opinion on this.

This is actually coming in #68

fjarri commented 1 week ago

I remember seeing your comment about tinyvec vs smallvec, and, having looked at them, I agree that tinyvec is preferable.