Skip to content
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

Refactor ecdsa structure to suit starkbank's pattern #21

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

xavier-stark
Copy link
Member

No description provided.

@xavier-stark xavier-stark force-pushed the refactor/ecdsa-structure branch from 6ccee2e to b1963e5 Compare December 6, 2022 19:44
@xavier-stark xavier-stark force-pushed the refactor/ecdsa-structure branch from b1963e5 to 8a612a3 Compare December 16, 2022 20:15
@xavier-stark xavier-stark force-pushed the refactor/ecdsa-structure branch from 8a612a3 to dc9182b Compare January 4, 2023 19:20
@@ -69,9 +79,9 @@ public static boolean verify(String message, Signature signature, PublicKey publ
return false;
}

BigInteger w = Math.inv(s, curve.N);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

linha 69 BigInteger.ONE

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ok

this(curve, RandomInteger.between(BigInteger.ONE, curve.N.subtract(BigInteger.ONE)));
}

public PrivateKey(BigInteger secret) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

acho que faltou a docstring em cima desses dois métodos

Object[] publicKeyStringObject = (Object[]) parsedObject1[1];
String publicKeyString = publicKeyStringObject[0].toString().toLowerCase();
if(privateKeyFlag != 1){
throw new Exception("Private keys should start with a '1' flag, but a " + privateKeyFlag + " was found instead");
}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

acho que valem uns newlines aqui e ali pra separar melhor os blocos lógicos

return publicKey;
}
if (p.isAtInfinity()) {
if(p.isAtInfinity()){
throw new RuntimeException("Public Key point is at infinity");
}
if (!curve.contains(p)) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

melhor padronizar se vai ser if() ou if () pra todo mundo logo

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ok

);
}

public static Signature _fromString(String string, BigInteger recoveryId) throws Exception {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

pq _fromString em vez de fromString? Não faz mais sentido deixar private logo?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ok

s = ((numberMessage.add(r.multiply(privateKey.secret))).multiply(Math.inv(randNum, curve.N))).mod(curve.N);
}
BigInteger recoveryId = randomSignPoint.y.and(BigInteger.ONE);
if (randomSignPoint.y.compareTo(curve.N) > 0){
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

randomSignPoint.x.compareTo(curve.N) > 0

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants