-
Notifications
You must be signed in to change notification settings - Fork 39
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
Change validator key to BLS12-381 #103
Conversation
@brunoffranca, does that mean that the bn254 curve will not be used at all? |
It will be used to sign L1 batches. L2 blocks and all consensus messages will use BLS12-381. |
@pompon0 I added two more commits removing |
@brunoffranca the point of having MsgHash was to avoid passing the whole message from place to place, and to avoid cloning as the messages can get quite large. I agree that so far we have underutilized those optimizations, but I can already see that you had to add a couple of extra clones in this PR already. |
@pompon0 Do you believe that this extra cloning does make a performance difference? And that we will actually start utilizing this MsgHash construct more in the future? If yes, I can remove these two commits. |
It is hard for me to tell. We can reintroduce it later, once there is a need to do some serious profiling. Just please avoid introducing |
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.
added some comments
also FYI, if we reintroduce it later it will be a breaking change to the protocol. |
Ok, like I always say, tie break goes to the reviewer. So I'll remove these two commits. |
010f6cd
to
5c8537a
Compare
What ❔
Changes the validator keys to use the BLS12-381 curve.
Why ❔
Now that we decided to separate validator and attester keys, we can use a more efficient curve for the validator keys.