-
Notifications
You must be signed in to change notification settings - Fork 1
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Protocol combinators #60
Conversation
bbf4b82
to
a006b37
Compare
Pull Request Test Coverage Report for Build 11823590883Details
💛 - Coveralls |
369068b
to
9922b66
Compare
@@ -149,14 +149,15 @@ struct Round1Payload { | |||
x: u8, | |||
} | |||
|
|||
impl<Id: PartyId> FirstRound<Id> for Round1<Id> { | |||
impl<Id: PartyId> EntryPoint<Id> for Round1<Id> { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Naming ideas:
- Init
- Start
- Gateway
- ProtocolStart
- InitRound
- Kickoff
Can you elaborate a bit on why "FirstRound" is a bad name? I kind of liked it because it's very descriptive.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It was logical when FirstRound
was bound on Round
, but it's not anymore. Now it's more like a Round factory.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This will be especially true if we make it stateful (see the note in the PR description).
b69876d
to
b9c3fcd
Compare
1. Define a type that will serve as the inputs for the new joined protocol. | ||
Most likely it will be a union between inputs of the first and the second protocol. | ||
|
||
2. Implement [`Chained`] for a type of your choice. Usually it will be an empty token type. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
"empty token type": you mean like a ZST e.g. struct ProtoC
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yep. Do you think "zero-sized type" would be more understandable here? Technically, it doesn't have to be zero-sized, it's just never instantiated by the crate.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think "token" is the correct term to use here, but given the world we live in, I suspect readers may confuse "token" with "cryptocurrency token".
Given you have a "usually" here, it doesn't have to be technically exact, it's more of a suggestion.
How about "Usually it will be a (possibly zero-sized) marker type."? Otherwise "token type" is fine too.
I'm struggling with this. I don't love
I like this!
I like
Hmm. I like the idea of a "splitting" trait, but I don't have a strong opinion on this. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This LGTM. I left a few more thoughts&nitpicks – feel free to make the changes or not and then merge.
This is actually coming in #68 |
I remember seeing your comment about |
We have several different situations where we need to build on top of an existing protocol or combine several of them:
Round::Protocol
the same. This is useful for writing tests.zip
combinator for protocols #61, will be implemented in another PR.extend
combinator for protocols. #62, will be implemented in another PR.misbehave
. Override macro and its machinery is removed fromtesting
.chain
.Also:
FirstRound
trait toEntryPoint
. It is not bound onRound
anymore.Protocol
type andENTRY_ROUND
constant toEntryPoint
.EntryPoint
andFinalizeOutcome::AnotherRound
now use a newBoxedRound
wrapper type.ProtocolError
for()
for protocols that don't use errors.CorrectnessProof
trait.RoundId
now supports nesting.Notes:
EntryPoint::new
take&self
? This way we can basically formalize the concept of protocol constructors used insynedrion
.misbehave
go intotesting
? Or gated behind thetesting
feature?EntryPoint::ENTRY_ROUND
, or make the user define it explicitly every time?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.From
for the type from the original protocol, which may be foreign in practice.