2 Commits

Author SHA1 Message Date
Juan Leni
853dd34d31 privval: improve Remote Signer implementation (#3351)
This issue is related to #3107
This is a first renaming/refactoring step before reworking and removing heartbeats.
As discussed with @Liamsi , we preferred to go for a couple of independent and separate PRs to simplify review work.

The changes:

    Help to clarify the relation between the validator and remote signer endpoints
    Differentiate between timeouts and deadlines
    Prepare to encapsulate networking related code behind RemoteSigner in the next PR

My intention is to separate and encapsulate the "network related" code from the actual signer.

SignerRemote ---(uses/contains)--> SignerValidatorEndpoint <--(connects to)--> SignerServiceEndpoint ---> SignerService (future.. not here yet but would like to decouple too)

All reconnection/heartbeat/whatever code goes in the endpoints. Signer[Remote/Service] do not need to know about that.

I agree Endpoint may not be the perfect name. I tried to find something "Go-ish" enough. It is a common name in go-kit, kubernetes, etc.

Right now:
SignerValidatorEndpoint:

    handles the listener
    contains SignerRemote
    Implements the PrivValidator interface
    connects and sets a connection object in a contained SignerRemote
    delegates PrivValidator some calls to SignerRemote which in turn uses the conn object that was set externally

SignerRemote:

    Implements the PrivValidator interface
    read/writes from a connection object directly
    handles heartbeats

SignerServiceEndpoint:

    Does most things in a single place
    delegates to a PrivValidator IIRC.

* cleanup

* Refactoring step 1

* Refactoring step 2

* move messages to another file

* mark for future work / next steps

* mark deprecated classes in docs

* Fix linter problems

* additional linter fixes
2019-02-28 11:48:20 +04:00
Ismail Khoffi
a6011c007d Close and retry a RemoteSigner on err (#2923)
* Close and recreate a RemoteSigner on err

* Update changelog

* Address Anton's comments / suggestions:

 - update changelog
 - restart TCPVal
 - shut down on `ErrUnexpectedResponse`

* re-init remote signer client with fresh connection if Ping fails

- add/update TODOs in secret connection
- rename tcp.go -> tcp_client.go, same with ipc to clarify their purpose

* account for `conn returned by waitConnection can be `nil`

- also add TODO about RemoteSigner conn field

* Tests for retrying: IPC / TCP

 - shorter info log on success
 - set conn and use it in tests to close conn

* Tests for retrying: IPC / TCP

 - shorter info log on success
 - set conn and use it in tests to close conn
 - add rwmutex for conn field in IPC

* comments and doc.go

* fix ipc tests. fixes #2677

* use constants for tests

* cleanup some error statements

* fixes #2784, race in tests

* remove print statement

* minor fixes from review

* update comment on sts spec

* cosmetics

* p2p/conn: add failing tests

* p2p/conn: make SecretConnection thread safe

* changelog

* IPCVal signer refactor

- use a .reset() method
- don't use embedded RemoteSignerClient
- guard RemoteSignerClient with mutex
- drop the .conn
- expose Close() on RemoteSignerClient

* apply IPCVal refactor to TCPVal

* remove mtx from RemoteSignerClient

* consolidate IPCVal and TCPVal, fixes #3104

- done in tcp_client.go
- now called SocketVal
- takes a listener in the constructor
- make tcpListener and unixListener contain all the differences

* delete ipc files

* introduce unix and tcp dialer for RemoteSigner

* rename files

- drop tcp_ prefix
- rename priv_validator.go to file.go

* bring back listener options

* fix node

* fix priv_val_server

* fix node test

* minor cleanup and comments
2019-01-13 14:31:31 -05:00