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
This commit is contained in:
Juan Leni
2019-02-28 08:48:20 +01:00
committed by Anton Kaliaev
parent d6e2fb453d
commit 853dd34d31
26 changed files with 1246 additions and 1152 deletions

View File

@@ -49,7 +49,7 @@ var _ error = (*TestHarnessError)(nil)
// with this version of Tendermint.
type TestHarness struct {
addr string
spv *privval.SocketVal
spv *privval.SignerValidatorEndpoint
fpv *privval.FilePV
chainID string
acceptRetries int
@@ -314,7 +314,7 @@ func (th *TestHarness) Shutdown(err error) {
// newTestHarnessSocketVal creates our client instance which we will use for
// testing.
func newTestHarnessSocketVal(logger log.Logger, cfg TestHarnessConfig) (*privval.SocketVal, error) {
func newTestHarnessSocketVal(logger log.Logger, cfg TestHarnessConfig) (*privval.SignerValidatorEndpoint, error) {
proto, addr := cmn.ProtocolAndAddress(cfg.BindAddr)
if proto == "unix" {
// make sure the socket doesn't exist - if so, try to delete it
@@ -334,20 +334,20 @@ func newTestHarnessSocketVal(logger log.Logger, cfg TestHarnessConfig) (*privval
switch proto {
case "unix":
unixLn := privval.NewUnixListener(ln)
privval.UnixListenerAcceptDeadline(cfg.AcceptDeadline)(unixLn)
privval.UnixListenerConnDeadline(cfg.ConnDeadline)(unixLn)
privval.UnixListenerTimeoutAccept(cfg.AcceptDeadline)(unixLn)
privval.UnixListenerTimeoutReadWrite(cfg.ConnDeadline)(unixLn)
svln = unixLn
case "tcp":
tcpLn := privval.NewTCPListener(ln, cfg.SecretConnKey)
privval.TCPListenerAcceptDeadline(cfg.AcceptDeadline)(tcpLn)
privval.TCPListenerConnDeadline(cfg.ConnDeadline)(tcpLn)
privval.TCPListenerTimeoutAccept(cfg.AcceptDeadline)(tcpLn)
privval.TCPListenerTimeoutReadWrite(cfg.ConnDeadline)(tcpLn)
logger.Info("Resolved TCP address for listener", "addr", tcpLn.Addr())
svln = tcpLn
default:
logger.Error("Unsupported protocol (must be unix:// or tcp://)", "proto", proto)
return nil, newTestHarnessError(ErrInvalidParameters, nil, fmt.Sprintf("Unsupported protocol: %s", proto))
}
return privval.NewSocketVal(logger, svln), nil
return privval.NewSignerValidatorEndpoint(logger, svln), nil
}
func newTestHarnessError(code int, err error, info string) *TestHarnessError {

View File

@@ -84,7 +84,7 @@ func TestRemoteSignerTestHarnessMaxAcceptRetriesReached(t *testing.T) {
func TestRemoteSignerTestHarnessSuccessfulRun(t *testing.T) {
harnessTest(
t,
func(th *TestHarness) *privval.RemoteSigner {
func(th *TestHarness) *privval.SignerServiceEndpoint {
return newMockRemoteSigner(t, th, th.fpv.Key.PrivKey, false, false)
},
NoError,
@@ -94,7 +94,7 @@ func TestRemoteSignerTestHarnessSuccessfulRun(t *testing.T) {
func TestRemoteSignerPublicKeyCheckFailed(t *testing.T) {
harnessTest(
t,
func(th *TestHarness) *privval.RemoteSigner {
func(th *TestHarness) *privval.SignerServiceEndpoint {
return newMockRemoteSigner(t, th, ed25519.GenPrivKey(), false, false)
},
ErrTestPublicKeyFailed,
@@ -104,7 +104,7 @@ func TestRemoteSignerPublicKeyCheckFailed(t *testing.T) {
func TestRemoteSignerProposalSigningFailed(t *testing.T) {
harnessTest(
t,
func(th *TestHarness) *privval.RemoteSigner {
func(th *TestHarness) *privval.SignerServiceEndpoint {
return newMockRemoteSigner(t, th, th.fpv.Key.PrivKey, true, false)
},
ErrTestSignProposalFailed,
@@ -114,15 +114,15 @@ func TestRemoteSignerProposalSigningFailed(t *testing.T) {
func TestRemoteSignerVoteSigningFailed(t *testing.T) {
harnessTest(
t,
func(th *TestHarness) *privval.RemoteSigner {
func(th *TestHarness) *privval.SignerServiceEndpoint {
return newMockRemoteSigner(t, th, th.fpv.Key.PrivKey, false, true)
},
ErrTestSignVoteFailed,
)
}
func newMockRemoteSigner(t *testing.T, th *TestHarness, privKey crypto.PrivKey, breakProposalSigning bool, breakVoteSigning bool) *privval.RemoteSigner {
return privval.NewRemoteSigner(
func newMockRemoteSigner(t *testing.T, th *TestHarness, privKey crypto.PrivKey, breakProposalSigning bool, breakVoteSigning bool) *privval.SignerServiceEndpoint {
return privval.NewSignerServiceEndpoint(
th.logger,
th.chainID,
types.NewMockPVWithParams(privKey, breakProposalSigning, breakVoteSigning),
@@ -135,7 +135,7 @@ func newMockRemoteSigner(t *testing.T, th *TestHarness, privKey crypto.PrivKey,
}
// For running relatively standard tests.
func harnessTest(t *testing.T, rsMaker func(th *TestHarness) *privval.RemoteSigner, expectedExitCode int) {
func harnessTest(t *testing.T, rsMaker func(th *TestHarness) *privval.SignerServiceEndpoint, expectedExitCode int) {
cfg := makeConfig(t, 100, 3)
defer cleanup(cfg)