-
Notifications
You must be signed in to change notification settings - Fork 1.1k
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
Added Milagro crypto-library with BLS12-381 implementation. #1236
Conversation
…t of message signing added.
@mkalinin ready for review |
resolves #1237 |
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.
Requesting more javadocs to the newly added classes
*/ | ||
public interface BLS381 { | ||
|
||
BI generatePrivate(); |
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 naming is a bit incomplete cause it doesn't point that generation has PRNG kind.
|
||
BI generatePrivate(); | ||
|
||
BI restorePrivate(BigInteger value); |
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 would abstract from any particular big number implementation. Wouldn't byte[]
representation be sufficient enough?
|
||
BI restorePrivate(BigInteger value); | ||
|
||
BI restorePrivate(byte[] value); |
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 seems that this method addresses more general case than restoring just private keys. Maybe it's better to call it restoreScalar
?
|
||
FP12Point pair(ECP2Point pointECP2, ECP1Point pointECP1); | ||
|
||
interface BI { |
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'd call it Scalar
FP12Point pair(ECP2Point pointECP2, ECP1Point pointECP1); | ||
|
||
interface BI { | ||
BigInteger asBigInteger(); |
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.
Should be removed in case if we're not sticking with any specific big number implementations
byte[] asByteArray(); | ||
} | ||
|
||
interface ECP1Point { |
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 ECP1
or even P1
would be pretty enough here
byte[] asByteArray(); | ||
} | ||
|
||
interface FP12Point {} |
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 is not a point, actually. Let's call it FP12
. And I would, also, explicitly add equals
method to that interface to point it out as an expecting one from underlying implementations
@@ -36,24 +44,30 @@ | |||
BigInteger aggPubs(List<BigInteger> pubKeys); | |||
|
|||
class Signature { | |||
public BigInteger r; | |||
public BigInteger s; | |||
public BigInteger value; |
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 doesn't look to me that having Signature
class is something useful. Maybe turn it into byte[]
?
@@ -36,24 +44,30 @@ | |||
BigInteger aggPubs(List<BigInteger> pubKeys); |
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 method looks redundant. pubKeys
may be passed to verify
directly.
...First test of message signing added.
Not ready for merge yet, a lot of work to go.