clean up

privval/messages.go

@@ -24,29 +24,29 @@ func RegisterRemoteSignerMsg(cdc *amino.Codec) {
 // PubKeyRequest requests the consensus public key from the remote signer.
 type PubKeyRequest struct{}
 
-// PubKeyResponse is a PrivValidatorSocket message containing the public key.
+// PubKeyResponse is a response message containing the public key.
 type PubKeyResponse struct {
 	PubKey crypto.PubKey
 	Error  *RemoteSignerError
 }
 
-// SignVoteRequest is a PrivValidatorSocket message containing a vote.
+// SignVoteRequest is a request to sign a vote
 type SignVoteRequest struct {
 	Vote *types.Vote
 }
 
-// SignedVoteResponse is a PrivValidatorSocket message containing a signed vote along with a potenial error message.
+// SignedVoteResponse is a response containing a signed vote or an error
 type SignedVoteResponse struct {
 	Vote  *types.Vote
 	Error *RemoteSignerError
 }
 
-// SignProposalRequest is a PrivValidatorSocket message containing a Proposal.
+// SignProposalRequest is a request to sign a proposal
 type SignProposalRequest struct {
 	Proposal *types.Proposal
 }
 
-// SignedProposalResponse is a PrivValidatorSocket message containing a proposal response
+// SignedProposalResponse is response containing a signed proposal or an error
 type SignedProposalResponse struct {
 	Proposal *types.Proposal
 	Error    *RemoteSignerError

privval/signer_client.go

@@ -10,7 +10,7 @@ import (
 )
 
 // SignerClient implements PrivValidator.
-// It uses a validator endpoint to request signatures from an external process.
+// Handles remote validator connections that provide signing services
 type SignerClient struct {
 	endpoint *SignerListenerEndpoint
 }
@@ -29,17 +29,17 @@ func NewSignerClient(endpoint *SignerListenerEndpoint) (*SignerClient, error) {
 	return &SignerClient{endpoint: endpoint}, nil
 }
 
-// Close calls Close on the underlying net.Conn.
+// Close closes the underlying connection
 func (sc *SignerClient) Close() error {
 	return sc.endpoint.Close()
 }
 
-// Close calls Close on the underlying net.Conn.
+// IsConnected indicates with the signer is connected to a remote signing service
 func (sc *SignerClient) IsConnected() bool {
 	return sc.endpoint.IsConnected()
 }
 
-// Close calls Close on the underlying net.Conn.
+// WaitForConnection waits maxWait for a connection or returns a timeout error
 func (sc *SignerClient) WaitForConnection(maxWait time.Duration) error {
 	if sc.endpoint == nil {
 		return fmt.Errorf("endpoint has not been defined")
@@ -50,7 +50,7 @@ func (sc *SignerClient) WaitForConnection(maxWait time.Duration) error {
 //--------------------------------------------------------
 // Implement PrivValidator
 
-// GetPubKey implements PrivValidator.
+// GetPubKey retrieves a public key from a remote signer
 func (sc *SignerClient) GetPubKey() crypto.PubKey {
 	response, err := sc.endpoint.SendRequest(&PubKeyRequest{})
 	if err != nil {
@@ -72,7 +72,7 @@ func (sc *SignerClient) GetPubKey() crypto.PubKey {
 	return pubKeyResp.PubKey
 }
 
-// SignVote implements PrivValidator.
+// SignVote requests a remote signer to sign a vote
 func (sc *SignerClient) SignVote(chainID string, vote *types.Vote) error {
 	sc.endpoint.Logger.Debug("SignerClient::SignVote")
 
@@ -95,7 +95,7 @@ func (sc *SignerClient) SignVote(chainID string, vote *types.Vote) error {
 	return nil
 }
 
-// SignProposal implements PrivValidator.
+// SignProposal requests a remote signer to sign a proposal
 func (sc *SignerClient) SignProposal(chainID string, proposal *types.Proposal) error {
 	response, err := sc.endpoint.SendRequest(&SignProposalRequest{Proposal: proposal})
 	if err != nil {

privval/signer_listener_endpoint.go

@@ -22,6 +22,7 @@ type SignerListenerEndpoint struct {
 	listener net.Listener
 	conn     net.Conn
 
+	timeoutAccept    time.Duration
 	timeoutReadWrite time.Duration
 
 	stopCh, stoppedCh chan struct{}
@@ -34,6 +35,7 @@ func NewSignerListenerEndpoint(logger log.Logger, listener net.Listener) *Signer
 
 	sc := &SignerListenerEndpoint{
 		listener:         listener,
+		timeoutAccept:    defaultTimeoutAcceptSeconds * time.Second,
 		timeoutReadWrite: defaultTimeoutReadWriteSeconds * time.Second,
 	}
 
@@ -76,7 +78,7 @@ func (sl *SignerListenerEndpoint) OnStop() {
 	<-sl.stoppedCh
 }
 
-// Close closes the underlying net.Conn.
+// Close closes the connection
 func (sl *SignerListenerEndpoint) Close() error {
 	sl.mtx.Lock()
 	defer sl.mtx.Unlock()
@@ -100,28 +102,24 @@ func (sl *SignerListenerEndpoint) WaitForConnection(maxWait time.Duration) error
 	return sl.ensureConnection(maxWait)
 }
 
-// SendRequest sends a request and waits for a response
+// SendRequest ensures there is a connection, sends a request and waits for a response
 func (sl *SignerListenerEndpoint) SendRequest(request RemoteSignerMsg) (RemoteSignerMsg, error) {
 	sl.mtx.Lock()
 	defer sl.mtx.Unlock()
 
-	// TODO: Add retries.. that include dropping the connection and
-
 	sl.Logger.Debug("SignerListenerEndpoint: Send request", "connected", sl.isConnected())
-	err := sl.ensureConnection(sl.timeoutReadWrite)
+	err := sl.ensureConnection(sl.timeoutAccept)
 	if err != nil {
 		return nil, err
 	}
 
 	sl.Logger.Debug("Send request. Write")
-
 	err = sl.writeMessage(request)
 	if err != nil {
 		return nil, err
 	}
 
 	sl.Logger.Debug("Send request. Read")
-
 	res, err := sl.readMessage()
 	if err != nil {
 		sl.Logger.Debug("Read Error", "err", err)
@@ -131,7 +129,6 @@ func (sl *SignerListenerEndpoint) SendRequest(request RemoteSignerMsg) (RemoteSi
 	return res, nil
 }
 
-// IsConnected indicates if there is an active connection
 func (sl *SignerListenerEndpoint) isConnected() bool {
 	return sl.IsRunning() && sl.conn != nil
 }
@@ -231,7 +228,6 @@ func (sl *SignerListenerEndpoint) ensureConnection(maxWait time.Duration) error
 	return nil
 }
 
-// dropConnection closes the current connection but does not touch the listening socket
 func (sl *SignerListenerEndpoint) dropConnection() {
 	if sl.conn != nil {
 		if err := sl.conn.Close(); err != nil {