-
Notifications
You must be signed in to change notification settings - Fork 78
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
dkg: exchange sigTypes concurrently #2496
Conversation
Before this commit, calling `exchange()` would listen for new incoming messages and discard everything that didn't carry the expected sigType. In a DKG one node might fall behind the others (for example, while DKG'ing 3000 validator keys) and send sigTypes that the other peers have already received threshold for. This commit makes sure a given instance of `exchanger` will hold onto all messages whose sigTypes are specified during its initialization phase. Calling `exchange()` will return as soon as there are enough messages for the specified sigType, but different sigTypes messages will still be stored and will be returned when requested by means of another `exchange()` call, with a different sigType argument.
Codecov ReportPatch coverage:
Additional details and impacted files@@ Coverage Diff @@
## main #2496 +/- ##
==========================================
+ Coverage 53.52% 53.70% +0.18%
==========================================
Files 198 199 +1
Lines 26757 26813 +56
==========================================
+ Hits 14321 14400 +79
+ Misses 10648 10626 -22
+ Partials 1788 1787 -1
☔ View full report in Codecov by Sentry. |
} | ||
|
||
// set sets data for the given sigType and core.PubKey. | ||
func (stb *dataByPubkey) set(pubKey core.PubKey, sigType sigType, data []core.ParSignedData) { |
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.
can refactor this to take a single sigdata
parameter
type sigTypeStore map[sigType]map[core.PubKey][]core.ParSignedData | ||
|
||
// dataByPubkey maps a sigType to its map of public key to slice of core.ParSignedData.. | ||
type dataByPubkey struct { |
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.
personal style: I would not introduce more types. I would just put the lock and a map[sigType]sigData
in exchanger
itself.
func (e exchanger) set(sigdata) {...}
func (e exchanger) get(sigtype) (sigdata, bool) {...}
But that is just me #lessismore
Avoids a goroutine and channel interaction
LGTM |
Before this commit, calling `exchange()` would listen for new incoming messages and discard everything that didn't carry the expected `sigType`. In a DKG one node might fall behind the others (for example, while DKG'ing 3000 validator keys) and send `sigTypes` that the other peers have already received threshold for. This PR makes sure a given instance of `exchanger` will hold onto all messages whose `sigTypes` are specified during its initialization phase. Calling `exchange()` will return as soon as there are enough messages for the specified `sigType`, but different `sigType`s messages will still be stored and will be returned when requested by means of another `exchange()` call, with a different `sigType` argument. category: bug ticket: #2491 Closes #2491.
Before this commit, calling
exchange()
would listen for new incoming messages and discard everything that didn't carry the expectedsigType
.In a DKG one node might fall behind the others (for example, while DKG'ing 3000 validator keys) and send
sigTypes
that the other peers have already received threshold for.This PR makes sure a given instance of
exchanger
will hold onto all messages whosesigTypes
are specified during its initialization phase.Calling
exchange()
will return as soon as there are enough messages for the specifiedsigType
, but differentsigType
s messages will still be stored and will be returned when requested by means of anotherexchange()
call, with a differentsigType
argument.category: bug
ticket: #2491
Closes #2491.