Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Closes #424
This adds a basic happy test for presigning, defines the characteristics that a valid set of
PresignRecord
s should satisfy, and adds a method to simulate a validpresign
run.There's lots of boilerplate to get the presign test running; this is mostly copied from the equivalent keygen and auxinfo tests. I think some of this could be extracted into a testing utility module because many of the functionalities (e.g. maintain an inbox to store and "send" messages) are the same across different participant types. I also think some of them could be simplified (e.g. we don't need separate inboxes for each party; we can just make one big inbox for all messages). But I refrain from addressing that here.
There's a bit of rearranging of code, like for the
simulate
methods on keygen and auxinfo, which I just moved into the test module so they're more obviously not-for-general-use. I'll mark the big chunks of moved code inline. I also had to add some functionality for simulating auxinfo and keygen output; before, we had methods to create a single valid instance; now we needed a method to create a valid set. Thesimulate
andsimulate_set
methods have a lot of similar code, but they take slightly different inputs and I didn't figure out a nice way to consolidate them.The most important cryptography component for @hridambasu to review is the tests on
PresignRecord
. The paper never explicitly defines what properties a record needs to hold. I generated the properties here by reading it and also reading @amaloz's excellent documentation on thePresignParticipant
type. But I would like you read the same things and think about whether there seem to be any other conditions that should hold for correctly-generated records.One other point of interest: in testing some of the presign stuff, I realized that the
CurvePoint::multiply_by_scalar
method is actually multiplying by aBigNumber
which it converts to aScalar
. This raised a few code quality concerns:bn_to_scalar
method it uses has some cloning of the input; we often call this method on secrets so this is a potential point of leakage we should address𝔽_q
. The paper isn't explicit about what that means, but𝔽_q
is (by definition) the scalar field of the elliptic curve. I think it's worth looking through these types to see if some of ourBigNumber
s can / should actually bek256::Scalar
s.For the purposes of this PR, I just changed the name to
multiply_by_bignumber
and made a separate, infalliblemultiply_by_scalar
method to make the types line up more nicely. This is why there are so many changed files... sorry. But perhaps in the future we will find thatmultiply_by_scalar
is the correct method to be using and we just have some types wrong.