[EFM] Protocol State represents DKG key vectors as mapping

[jordanschalm]

Problem Definition

General Context

Currently DKG committees are stored and represented in most places in our codebase as an ordered list of keys $K$ corresponding to a consensus committee $C$. There are equal numbers of keys and consensus committee members, and if $C$ is ordered according to canonical ordering, then $K[i]$ is the key for $C[i]$. The index of a node in the consensus committee is always equal to the index of that node's key in the DKG key list.

Let $D$ be the set of nodes that participated in the DKG yielding $K$. Our model works when $D=C$, but does not work in any other case.

• This representation does not map well to the underlying DKG index concept, where indices are meaningful and not necessarily consecutive.
• This representation cannot represent cases where:
  • $\exists n : n \in D, n \notin C$ - there is a node that participated in the DKG, but isn't in the committee
  • $\exists n : n \in C, n \notin D$ - there is a node that is in the committee, but didn't participate in the DKG

We would like to change the way DKG committees are represented to support the above cases.

New DKG Data Model

The updated data model should include the following fields:

• PubKeys - A list of all individual public keys, ordered by DKG index.
  • []crypto.PublicKey in Go
  • [String] in Cadence, each element being a hex-encoded string
• IndexMap - A mapping from node ID to DKG index (includes node IDs that do not exist in the consensus committee)
  • map[flow.Identifier]int in Go
  • {String:Int} in Cadence, keys being hex-encoded identifiers
• GroupKey - The single group public key. crypto.PublicKey in Go, String (hex-encoded) in Cadence.

It has the invariants:

• len(PubKeys) = len(IndexMap)
• IndexMap values form the set {0, 1, ..., n-1} where n=len(PubKeys)

Specifics for this Issue

Update the Protocol State's internal representation of DKG committees, its interactions with the crypto layer, and its interactions with the FlowDKG smart contract, to be compatible with the cases above.

Proposed Solution

• [EFM] Smart Contract represents DKG key vectors as mappings #6213 modifies FlowDKG transactions to accept DKG "final results" as an index mapping in addition to a key list. Update the DKGClient to send transactions using this representation
• [EFM] Smart Contract represents DKG key vectors as mappings #6213 modifies FlowEpoch.EpochCommit service event to use the mapping representation for DKG results.
  • Update the EpochCommit data model and service event conversion logic in flow-go to use a mapping representation
• When instantiating a Beacon Signer/Verifier, we need to construct the public keys list input based on the key list from key list (do not use consensus committee)
• When computing the threshold for a DKG, make sure to use the size of the DKG key list, NOT the size of the consensus committee!
• Update resetEpoch and efmRecovery transaction argument generator tools to return DKG key mappings

Definition of Done

The existing epoch integration tests should pass, after implementing this and #6213. We will test the cases of mismatched consensus committee and DKG committee as part of #5945.

Ideally we also create an integration test for the case where the committees are mismatched, in particular the case where a node is ejected/un-staked before the RecoveryEpoch.

Tasks

Preview Give feedback

1. [EFM] Modify DKG reactor engine and related logic to push IndexMap #6319
   Protocol S-Consensus +2
   
2. [EFM] Modify EpochCommit parsing logic to correctly convert Cadence structure #6320
   +0
3. [EFM] Modify implementors of protocol.DKG to use IndexMap #6321
   Protocol S-Consensus +2
   
4. [EFM] Create integration test where node is ejected before recovery #6331
   S-BFT S-Consensus +2
   
5. [EFM] Enable previously disabled integration tests #6469
   S-Consensus Tech Debt +2
   
6. [EFM] Modify EpochRecover parsing logic to correctly convert Candence structure #6495
   Protocol S-BFT +2
   

[github-actions]

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

[jordanschalm]

Only remaining issue is BN testing, closing...