-
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
Broadcast validators signature and collect QC (BFT-414) #76
Conversation
@IAvecilla I think it will be better if we create another validator key just for batch signing. So, this way we can use different signatures schemes for block signing and for batch signing. |
Statekeeper is storing blocks to postgres in parallel to processing next blocks. Our simplified API so far required persisting one block to finish before processing the next block. This effectively disabled the parallelization in statekeeper and therefore the EN with consensus enabled was significantly slower than EN without consensus. This PR updates the api to allow persisting multiple blocks in parallel. Long term this change most likely won't be that useful, because statekeeper of a validator node will have to work synchronously with consensus (but that requires improving statekeeper anyway). I've also implemented caching inmemory the most recent blocks (even if they got persisted) to address the inefficiency found during the loadtest.
remove -Dwarnings like here https://github.com/matter-labs/era-consensus/pull/114/files to fix the presubmit |
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 am sending some comments to ask some conceptual questions and to help me understand the code. Sorry to add more to the long comment history, I hope you don't mind.
As a general note:
- I see some TODOs left in the code, like this one. It would be helpful to state whether this PR completely implements what it says in the description, or there are things deferred to future PRs (perhaps with their ticket number).
- Perhaps this PR could have been delivered in two parts: the attestations model first, and then hooking them into the flow.
@aakoshh Most of your comments are valid and correct. However, the current logic won't work as intended because we don't have the syncing batches logic in place yet (which is in progress). This means that much of this code will change in the future once we can test it properly. This was just an initial implementation to see how the signature propagation would look. I think @pompon0 mentioned something similar while I was writing this. :) |
What ❔
Implement a mechanism for validators to broadcast their signatures to other nodes in the network and collect them in a new certificate.
Why ❔
This is essential for signing L1 batches and sending them to L1 for verification. Validators need to broadcast their signatures and gather them in a certificate in case the majority signs the new batch.