-
Notifications
You must be signed in to change notification settings - Fork 5
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
Implement interactive signing #475
Conversation
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.
Looks good to me!
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.
The Input type here needs a proper constructor. I was thinking of doing that in a separate PR just because this one is already big (I haven't written that issue yet).
Makes sense.
Same for removing the input
method.
Thoughts on making a bespoke little enum for that?
I agree! The enum makes sense for tracking the "state" and avoids the clone. (Even though, from what I can tell, this data isn't security critical). I tend to dislike Option
s when used as fields like in InteractiveSignParticipant
- Remove `input` method from protocol participant trait - Add protocol implementation that dispatches to presign and NI sign - Add a single happy test for interactive sign
47c17f2
to
163b16f
Compare
@gatoWololo I added said enum; there's a fairly ugly update method that I'd love your thoughts on. Updating a field in a mutable reference in-place is really awkward; I did it with a lot of |
Thanks @gatoWololo, I like your version much better. This is now ready for re-review. |
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.
Looks good!
Closes #467
This PR adds an implementation of interactive signing.
A couple discussion topics:
input
method onProtocolParticipant
wasn't being used in any protocol-participant-generic methods, so I removed it as a method on that trait. I did this because it doesn't make sense for this implementation to keep a separate copy of the private auxinfo and keygen material around in theInteractiveSignParticipant
; better to just pass it to the internalPresignParticipant
and wash our hands of it. However, that makes it hard to implement theinput()
method, so I removed it.Input
type here needs a proper constructor. I was thinking of doing that in a separate PR just because this one is already big (I haven't written that issue yet).