P2P communication improvements #1
Conversation
kevinssgh
commented
Mar 17, 2023
kevinssgh
commented
- Modified unit test
- Modified error handling for 3 communication components: p2p comms, signature notifier, party coordinator
| n, ok := s.notifiers[msg.ID] | ||
| if !ok { | ||
| logger.Debug().Msgf("notifier for message id(%s) not exist", msg.ID) | ||
| _ = stream.Close() |
There was a problem hiding this comment.
if stream was not closed here, then will it be eventually closed? @kevinssgh
There was a problem hiding this comment.
Yes only once all the goroutines have finished here:
https://github.com/brewmaster012/go-tss/blob/8f5e91b0ad5b8cd29caa05408edfcf44cd97dfff/tss/keysign.go#L282
The streams from the 3 components get released in the defer line here:
https://github.com/brewmaster012/go-tss/blob/8f5e91b0ad5b8cd29caa05408edfcf44cd97dfff/tss/keysign.go#L214
I believe the issue was with these streams being open unnecessarily for too long.
There was a problem hiding this comment.
hmm.. any idea as to why we have "deadlock" if the streams get eventually released?
is it because once stream is exhausted, the "release" functions get deadlocked somehow because no progress can be made otherwhere? @kevinssgh
There was a problem hiding this comment.
I'm not certain that it is exactly a deadlock. It seems like the P2P layer is blocking certain protocols while the streams are open for too long. Even though they are released eventually the lower level blocks the communication protocol.
|
Also what is the purpose of the "UNKNOWN" streams? Why are they not immediately closed? |
|
The purpose of those UNKNOWN streams is mainly for incoming connections that aren't associated with a party or message id. I noticed that there was a bug where they weren't deleted from the stream map, that was something else I fixed as well but not sure if it had a direct impact. |
