refactor: update/add committee data types (BFT-443) #163
Conversation
| @@ -198,4 +198,9 @@ message AggregateSignature { | |||
| message WeightedValidator { | |||
| optional PublicKey key = 1; // required | |||
| optional uint64 weight = 2; // required | |||
| optional Signature proof_of_possession = 3; // optional | |||
There was a problem hiding this comment.
Sorry, I got confused for a moment: so here you are introducing the new types for committees which are not placed in the Genesis. Also, afaict these types are supposed to be used solely on the zksync-era side, in which case you should define them in zksync-era, not here.
There was a problem hiding this comment.
let's discuss it on the standup
There was a problem hiding this comment.
WeightedValidator is placed in Genesis (part of validator::Committee), that's why I thought it's fine to make it optional, as part of the rollout (although it's not ideal).
The list types (ValidatorCommittee, etc.) are indeed about introducing a new type which isn't part of Genesis. In addition, They're simpler types, minimal for (de)serializing for DB store (AFAIK a simpler, Vec<WeightedValidator> isn't viable in protobuf).
As for the location in the codebase, since it will eventually need to be also passed in-mem from zksync-era to era-consensus, placing it here makes sense. Otherwise, another (de)serialization will need to happen.
|
Closing in favor of not introducing cross-repo integration, and instead having matter-labs/zksync-era#2518 to add temporary data type clones. |
What ❔
Adding minimally needed changes to support matter-labs/zksync-era#2518, as part of committees rotation (BFT-484).
pop: validator::Signature(proof-of-possession) field toWeightedValidator.ValidatorCommittee&AttesterCommitteetypes to provide protobuf formatting for DB store.Why ❔
These types cannot be placed in
zksync_node_consensusbecause they are required byzksync_dal::consensus_dal, which is a dependency ofzksync_node_consensus.