Priv val via sockets by ebuchman · Pull Request #1060 · tendermint/tendermint

ebuchman · 2018-01-05T00:53:17Z

Support a socket client and server for the priv validator.

Dead simple synchronous server.

@xla @ethanfrey want to work on this ?

see #1044 for more context

xla · 2018-01-05T13:19:55Z

@ebuchman Can take this on after wrapping up #1056

adrianbrink · 2018-01-08T12:08:14Z

implements #819

melekes · 2018-01-08T16:23:02Z

types/priv_validator/socket.go

+	}
+
+	var n int
+	b = wire.ReadByteSlice(conn, 0, &n, &err) //XXX: no max


check for err before calling decodeMsg

melekes · 2018-01-08T16:24:01Z

types/priv_validator/socket.go

+	n := new(int)
+	r := bytes.NewReader(bz)
+	msgI := wire.ReadBinary(struct{ PrivValidatorSocketMsg }{}, r, 0, n, &err)
+	msg = msgI.(struct{ PrivValidatorSocketMsg }).PrivValidatorSocketMsg


is msgI guaranteed to be PrivValidatorSocketMsg in case of error?

melekes · 2018-01-08T16:25:09Z

types/priv_validator/socket.go

+		pvss.Logger.Info("Accepted a new connection")
+
+		// read/write
+		for {


only one connection at a time?

@ebuchman Is one connection at a time by design, or should the server be able to hanlde multiple, if so are there any upper bounds we want to enforce, or any other limiting requirements?

melekes · 2018-01-08T16:27:13Z

types/priv_validator/socket.go

+}
+
+func (pvss *PrivValidatorSocketServer) OnStart() error {
+	if err := pvss.BaseService.OnStart(); err != nil {


BaseService#OnStart is empty. no need to call it.

melekes · 2018-01-08T16:27:29Z

types/priv_validator/socket.go

+}
+
+func (pvss *PrivValidatorSocketServer) OnStop() {
+	pvss.BaseService.OnStop()


BaseService#OnStop is empty. no need to call it.

melekes · 2018-01-08T16:28:59Z

types/priv_validator/socket.go

+		pvss.Logger.Error("Error closing listener", "err", err)
+	}
+
+	if err := pvss.conn.Close(); err != nil {


check if pvss.conn != nil

* add comments to all public types * fix comments to adhere to comment standards

ebuchman · 2018-01-17T16:36:58Z

Thanks Alex.

Couple notes here.

Server should be on the Tendermint side. The signer should dial Tendermint.
Should be able to handle multiple connections, but let's cap it at something reasonable (maybe 3).
Let's use go-wire for serialization
Let's use the p2p.SecretConnection for authentication

@tarcieri is working on the signer itself in Rust

xla · 2018-01-17T18:45:49Z

Thanks for follow-up.

Server should be on the Tendermint side. The signer should dial Tendermint.

Should the server exist as it's own binary under cmd/priv_validator or ultimately become a command of the tendermint cli?

Should be able to handle multiple connections, but let's cap it at something reasonable (maybe 3).

Seems like a good first baseline, maybe worth having it configurable.

@tarcieri is working on the signer itself in Rust

Going to sync with him to see if we can make each others work easier.

ebuchman · 2018-01-17T18:58:17Z

Should the server exist as it's own binary under cmd/priv_validator or ultimately become a command of the tendermint cli?

No it should run in-process when tendermint node boots up. A tendermint node always needs a priv validator - either it's given a priv_validator.json with a private key direcetly (ie. useful for dev/testing) or it starts an in-process listener and waits to receive a connection from an external signing process

Going to sync with him to see if we can make each others work easier.

Sounds great

xla · 2018-02-08T18:16:38Z

@ebuchman @melekes This is ready for review from my perspective. Tried to address all points.

When working with the protocol between client and server I was dearly missing some form of communicating errors, but I don't know how settle we are on the current protocol.

ebuchman · 2018-02-09T22:07:27Z

Thanks Alex. I've moved this over to a fresh PR by rebasing on a fresh copy of the new validator code without integrating to the rest of the codebase. Let's take up review/discussion there

wip: priv val via sockets

49b3b38

ebuchman requested a review from melekes as a code owner January 5, 2018 00:53

melekes reviewed Jan 8, 2018

View reviewed changes

xla self-assigned this Jan 17, 2018

Alexander Simmerl added 2 commits January 17, 2018 17:12

wip: Comment types

7c24c0c

* add comments to all public types * fix comments to adhere to comment standards

wip: fix code block in ADR

6c63adf

xla force-pushed the priv_val_socket branch from ccf8cd9 to 6c63adf Compare January 17, 2018 16:22

Alexander Simmerl added 3 commits January 18, 2018 19:23

wip: fix nil pointer deference

e8905d8

wip: Avoid underscore in var name

802f6ee

wip: check error of wire read

eccf88c

This was referenced Jan 21, 2018

Detached Signing Validator Design #819

Closed

Active/active validator setup #870

Closed

xla added 5 commits January 25, 2018 16:25

Correct server protocol

57b4dd9

Use secret connection

0099380

Format and consolidate

02ffede

Harden tests

dde9852

Integrate privVal client with node secret

07783a3

melekes changed the title ~~wip: priv val via sockets~~ Priv val via sockets Feb 9, 2018

ebuchman mentioned this pull request Feb 9, 2018

Feature/priv val sockets #1204

Merged

ebuchman closed this Feb 9, 2018

xla mentioned this pull request Feb 28, 2018

types/priv_validator package #1203

Merged

xla deleted the priv_val_socket branch September 18, 2018 20:27

Conversation

ebuchman commented Jan 5, 2018

Uh oh!

xla commented Jan 5, 2018

Uh oh!

adrianbrink commented Jan 8, 2018

Uh oh!

melekes Jan 8, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

melekes Jan 8, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

melekes Jan 8, 2018

Choose a reason for hiding this comment

Uh oh!

xla Jan 17, 2018

Choose a reason for hiding this comment

Uh oh!

melekes Jan 8, 2018

Choose a reason for hiding this comment

Uh oh!

melekes Jan 8, 2018

Choose a reason for hiding this comment

Uh oh!

melekes Jan 8, 2018

Choose a reason for hiding this comment

Uh oh!

ebuchman commented Jan 17, 2018

Uh oh!

xla commented Jan 17, 2018

Uh oh!

ebuchman commented Jan 17, 2018

Uh oh!

xla commented Feb 8, 2018

Uh oh!

ebuchman commented Feb 9, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

melekes Jan 8, 2018 •

edited

Loading

melekes Jan 8, 2018 •

edited

Loading

ebuchman commented Feb 9, 2018 •

edited

Loading