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

Remove hard coded elliptic curve lengths #1967

Conversation

daniel-iobuilders
Copy link
Contributor

Signed-off-by: Daniel Lehrner daniel@io.builders

As discussed in #1933, the byte size for different variables of the elliptic curve cryptography are hard coded and don't allow the future implementation of algorithms other than SECP256*. This PR removes all these hard coded byte sizes and replaces them by dynamic ones. This allows the future implementation of algorithms like SECP384R1 (NIST P-384).
This is a simple refactoring, no existing functionality was modified or removed, neither was a new one added.

…hs and elliptic curves.

Signed-off-by: Daniel Lehrner <daniel@io.builders>
Signed-off-by: Daniel Lehrner <daniel@io.builders>
@daniel-iobuilders daniel-iobuilders changed the title Remove hard ec lengths Remove hard coded elliptic curve lengths Mar 2, 2021
@@ -69,7 +69,9 @@ private static Bytes serializeHeaderWithoutProposerSeal(
private static Bytes encodeExtraDataWithoutProposerSeal(final CliqueExtraData cliqueExtraData) {
final Bytes extraDataBytes = cliqueExtraData.encode();
// Always trim off final 65 bytes (which maybe zeros)
Copy link
Member

Choose a reason for hiding this comment

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

The comment needs to be changed to reflect it might not be 65 anymore :)

final byte recId = bytes.get(64);
return SECPSignature.create(r, s, recId, curveOrder);
bytes.size() == signatureLength,
new StringBuilder()
Copy link
Member

Choose a reason for hiding this comment

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

We are missing the spaces between the appends, aren't we?

@lucassaldanha lucassaldanha self-assigned this Mar 4, 2021
Signed-off-by: Daniel Lehrner <daniel@io.builders>
Signed-off-by: Daniel Lehrner <daniel@io.builders>
Signed-off-by: Daniel Lehrner <daniel@io.builders>
Signed-off-by: Daniel Lehrner <daniel@io.builders>
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.

2 participants