-
Notifications
You must be signed in to change notification settings - Fork 2.1k
Description
Tendermint version: 0.34.24
While diving into the privval package source code as part of an investigation into horcrux we discovered that the "accept connection logic" is unclear.
See source. Writing to the unbuffered channel sl.connectRequestCh after attempting to accept a connection in SignerListenerEndpoint.serviceLoop has no affect. This is because a single goroutine cannot read and write to the same unbuffered channel, it can only do that with buffered channels.
It would therefore be clearer to remove that select statement since it never actually writes to the channel.
I am also a bit uncertain about the reasoning behind the serviceLoop, is it
a) for each connectRequest, successfully accept one connection, or
b) continuously accept new connections?
If a), then one could changeconnectRequestCh into a buffered channel with length 1 and only write to it when acceptNewConnection errored.
If b), then connectRequestCh can be removed and replaced with just a for loop. This will however block when writing to connectionAvailableCh since the reader on the other side doesn't read when it already has an active connection.