DKG: Handle nil key vector submissions
Closed this issue · 8 comments
Context
When the DKG fails, it returns a key vector containing nil keys. Despite the failure, these key vectors should still be submitted to the smart contract. When deciding whether the DKG has completed, we should consider a key vector (final submission) containing any nil keys to be invalid, even if a majority of participants have submitted that key vector.
Definition of Done
- Decide how to serialize a nil key (either change smart contract to accept
Array<Optional<String>>and encode nil keys as nil or define a canonical serialization for a nil key (eg. 0x00...) coordinate with https://github.com/dapperlabs/flow-go/issues/5452 - Update key vector submission / storage to use updated encoding, if necessary
- Modify logic determining when DKG is done to consider case where majority key vector contains nil keys as the DKG not being done
So right now, we need to discuss if we want to define a nil-key as 0x0000...(len=192) or just as nil?
If we define it as nil, will the 0x0000... case still be a potential problem, or can we just not allow that in the node software?
I think making the submissions optionals is probably the better choice, but it seems kind of arbitrary to me.
Should we not allow those nil submissions at all, or just say that the DKG isn't complete if all the keys are nil?
Should we not allow those nil submissions at all, or just say that the DKG isn't complete if all the keys are nil?
We should accept nil submissions and say the DKG isn't complete if a nil submission "wins" (exceeds the threshold - >50% of nodes submit this key vector)
So right now, we need to discuss if we want to define a nil-key as 0x0000...(len=192) or just as nil?
We probably don't really need to discuss it much, we just need to pick one. I would agree using optionals is the better approach, so let's go with that 👍
Should we also define 0x00000000.... as invalid? Or is that still a valid key?
If we're saying nil represents a nil key, I don't think we need to special-case 0x00.... We can just expect it won't be submitted since honest protocol software won't serialize it that way.
As an invalid input from a Byzantine DKG participant, we would eventually be able to detect that by cryptographically verifying the key vector submissions.
Should we also define 0x00000000.... as invalid? Or is that still a valid key?
The contract should not check the validity of the submissions at this stage. The contract should only do the count. Assuming there is a honest majority, the result that wins the count has to be valid. (and it is later checked before being used anyway).
If you decide that a nil key is gonna be represented by a string of 192 characters, I prefer not to use 0x00.., I'll provide another constant.
@jordanschalm @tarakby So we have stated that a vector of all nil keys is valid for a submission, but it is invalid for determining if the dkg is complete, correct?
What if only some of the keys are nil, but others are valid keys? What do we do there? I'm assuming that if any key is nil it is still a valid submission, but it doesn't count as completing the dkg if all the nodes submitted vectors with some nil keys
I think the logic should look like this:
if one key vector submission `S` exceeds threshold:
if any key in `S` is nil:
the DKG has failed - we never consider this DKG instance complete and never emit the `EpochCommit` event
else:
`S` is the canonical DKG output for this instance, we consider this DKG instance complete
So we have stated that a vector of all nil keys is valid for a submission, but it is invalid for determining if the dkg is complete, correct?
Once a submission reaches the threshold number, the contract looks at its keys. If they are all non nil keys, the contract can declare that DKG has completed successfully through an event. If any key is nil, it's safe to say DKG has failed, but the contract will just not emit any event.
( @jordan has beaten me to this )