json2wal: increase reader's buffer size (#3147)

```
panic: failed to unmarshal json: unexpected end of JSON input

goroutine 1 [running]:
main.main()
	/root/gelgo/src/github.com/tendermint/tendermint/scripts/json2wal/main.go:66 +0x73f
```

Closes #3146

CHANGELOG.md

@@ -1,5 +1,60 @@
 # Changelog
 
+## v0.28.0
+
+*January 16th, 2019*
+
+Special thanks to external contributors on this release:
+@fmauricios, @gianfelipe93, @husio, @needkane, @srmo, @yutianwu
+
+This release is primarily about upgrades to the `privval` system -
+separating the `priv_validator.json` into distinct config and data files, and
+refactoring the socket validator to support reconnections.
+
+**Note:** Please backup your existing `priv_validator.json` before using this
+version.
+
+See [UPGRADING.md](UPGRADING.md) for more details.
+
+### BREAKING CHANGES:
+
+* CLI/RPC/Config
+  - [cli] Removed `--proxy_app=dummy` option. Use `kvstore` (`persistent_kvstore`) instead.
+  - [cli] Renamed `--proxy_app=nilapp` to `--proxy_app=noop`.
+  - [config] [\#2992](https://github.com/tendermint/tendermint/issues/2992) `allow_duplicate_ip` is now set to false
+  - [privval] [\#1181](https://github.com/tendermint/tendermint/issues/1181) Split `priv_validator.json` into immutable (`config/priv_validator_key.json`) and mutable (`data/priv_validator_state.json`) parts (@yutianwu)
+  - [privval] [\#2926](https://github.com/tendermint/tendermint/issues/2926) Split up `PubKeyMsg` into `PubKeyRequest` and `PubKeyResponse` to be consistent with other message types
+  - [privval] [\#2923](https://github.com/tendermint/tendermint/issues/2923) Listen for unix socket connections instead of dialing them
+
+* Apps
+
+* Go API
+  - [types] [\#2981](https://github.com/tendermint/tendermint/issues/2981) Remove `PrivValidator.GetAddress()`
+
+* Blockchain Protocol
+
+* P2P Protocol
+
+### FEATURES:
+- [rpc] [\#3052](https://github.com/tendermint/tendermint/issues/3052) Include peer's remote IP in `/net_info`
+
+### IMPROVEMENTS:
+- [consensus] [\#3086](https://github.com/tendermint/tendermint/issues/3086) Log peerID on ignored votes (@srmo)
+- [docs] [\#3061](https://github.com/tendermint/tendermint/issues/3061) Added specification for signing consensus msgs at
+  ./docs/spec/consensus/signing.md
+- [privval] [\#2948](https://github.com/tendermint/tendermint/issues/2948) Memoize pubkey so it's only requested once on startup
+- [privval] [\#2923](https://github.com/tendermint/tendermint/issues/2923) Retry RemoteSigner connections on error
+
+### BUG FIXES:
+
+- [build] [\#3085](https://github.com/tendermint/tendermint/issues/3085) Fix `Version` field in build scripts (@husio)
+- [crypto/multisig] [\#3102](https://github.com/tendermint/tendermint/issues/3102) Fix multisig keys address length
+- [crypto/encoding] [\#3101](https://github.com/tendermint/tendermint/issues/3101) Fix `PubKeyMultisigThreshold` unmarshalling into `crypto.PubKey` interface
+- [p2p/conn] [\#3111](https://github.com/tendermint/tendermint/issues/3111) Make SecretConnection thread safe
+- [rpc] [\#3053](https://github.com/tendermint/tendermint/issues/3053) Fix internal error in `/tx_search` when results are empty
+  (@gianfelipe93)
+- [types] [\#2926](https://github.com/tendermint/tendermint/issues/2926) Do not panic if retrieving the privval's public key fails
+
 ## v0.27.4
 
 *December 21st, 2018*
@@ -17,9 +72,8 @@
 ### BREAKING CHANGES:
 
 * Go API
-
-- [dep] [\#3027](https://github.com/tendermint/tendermint/issues/3027) Revert to mainline Go crypto library, eliminating the modified
-  `bcrypt.GenerateFromPassword`
+  - [dep] [\#3027](https://github.com/tendermint/tendermint/issues/3027) Revert to mainline Go crypto library, eliminating the modified
+    `bcrypt.GenerateFromPassword`
 
 ## v0.27.2
 

CHANGELOG_PENDING.md

@@ -1,4 +1,4 @@
-## v0.27.4
+## v0.29.0
 
 *TBD*
 
@@ -7,28 +7,23 @@ Special thanks to external contributors on this release:
 ### BREAKING CHANGES:
 
 * CLI/RPC/Config
-- [cli] Removed `node` `--proxy_app=dummy` option. Use `kvstore` (`persistent_kvstore`) instead.
-- [cli] Renamed `node` `--proxy_app=nilapp` to `--proxy_app=noop`.
-- [config] \#2992 `allow_duplicate_ip` is now set to false
-
-- [privval] \#2926 split up `PubKeyMsg` into `PubKeyRequest` and `PubKeyResponse` to be consistent with other message types
+- [types] consistent field order of `CanonicalVote` and `CanonicalProposal`
 
 * Apps
 
-* Go API  
-- [types] \#2926 memoize consensus public key on initialization of remote signer and return the memoized key on 
-`PrivValidator.GetPubKey()` instead of requesting it again 
-- [types] \#2981 Remove `PrivValidator.GetAddress()`
+* Go API
+- [node] \#3082 MetricsProvider now requires you to pass a chain ID
 
 * Blockchain Protocol
+  * [merkle] \#2713 Merkle trees now match the RFC 6962 specification
 
 * P2P Protocol
-- multiple connections from the same IP are now disabled by default (see `allow_duplicate_ip` config option)
 
 ### FEATURES:
-- [privval] \#1181 Split immutable and mutable parts of priv_validator.json
 
 ### IMPROVEMENTS:
+- [rpc] \#3065 return maxPerPage (100), not defaultPerPage (30) if `per_page` is greater than the max 100.
+- [instrumentation] \#3082 add 'chain_id' label for all metrics
 
 ### BUG FIXES:
-- [types] \#2926 do not panic if retrieving the private validator's public key fails
+- [log] \#3060 fix year format

Makefile

@@ -292,9 +292,7 @@ build-linux:
 	GOOS=linux GOARCH=amd64 $(MAKE) build
 
 build-docker-localnode:
-	cd networks/local
-	make
-	cd -
+	@cd networks/local && make
 
 # Run a 4-node testnet locally
 localnet-start: localnet-stop

README.md

@@ -1,8 +1,8 @@
 # Tendermint
 
 [Byzantine-Fault Tolerant](https://en.wikipedia.org/wiki/Byzantine_fault_tolerance)
-[State Machine Replication](https://en.wikipedia.org/wiki/State_machine_replication).
-Or [Blockchain](https://en.wikipedia.org/wiki/Blockchain_(database)) for short.
+[State Machines](https://en.wikipedia.org/wiki/State_machine_replication).
+Or [Blockchain](https://en.wikipedia.org/wiki/Blockchain_(database)), for short.
 
 [![version](https://img.shields.io/github/tag/tendermint/tendermint.svg)](https://github.com/tendermint/tendermint/releases/latest)
 [![API Reference](
@@ -66,49 +66,26 @@ See the [install instructions](/docs/introduction/install.md)
 - [Remote cluster using terraform and ansible](/docs/networks/terraform-and-ansible.md)
 - [Join the Cosmos testnet](https://cosmos.network/testnet)
 
-## Resources
-
-### Tendermint Core
-
-For details about the blockchain data structures and the p2p protocols, see the
-the [Tendermint specification](/docs/spec).
-
-For details on using the software, see the [documentation](/docs/) which is also
-hosted at: https://tendermint.com/docs/
-
-### Tools
-
-Benchmarking and monitoring is provided by `tm-bench` and `tm-monitor`, respectively.
-Their code is found [here](/tools) and these binaries need to be built seperately.
-Additional documentation is found [here](/docs/tools).
-
-### Sub-projects
-
-* [Amino](http://github.com/tendermint/go-amino), a reflection-based improvement on proto3
-* [IAVL](http://github.com/tendermint/iavl), Merkleized IAVL+ Tree implementation
-
-### Applications
-
-* [Cosmos SDK](http://github.com/cosmos/cosmos-sdk); a cryptocurrency application framework
-* [Ethermint](http://github.com/cosmos/ethermint); Ethereum on Tendermint
-* [Many more](https://tendermint.com/ecosystem)
-
-### Research
-
-* [The latest gossip on BFT consensus](https://arxiv.org/abs/1807.04938)
-* [Master's Thesis on Tendermint](https://atrium.lib.uoguelph.ca/xmlui/handle/10214/9769)
-* [Original Whitepaper](https://tendermint.com/static/docs/tendermint.pdf)
-* [Blog](https://blog.cosmos.network/tendermint/home)
-
 ## Contributing
 
-Yay open source! Please see our [contributing guidelines](CONTRIBUTING.md).
+Please abide by the [Code of Conduct](CODE_OF_CONDUCT.md) in all interactions,
+and the [contributing guidelines](CONTRIBUTING.md) when submitting code.
+
+Join the larger community on the [forum](https://forum.cosmos.network/) and the [chat](https://riot.im/app/#/room/#tendermint:matrix.org).
+
+To learn more about the structure of the software, watch the [Developer
+Sessions](https://www.youtube.com/playlist?list=PLdQIb0qr3pnBbG5ZG-0gr3zM86_s8Rpqv)
+and read some [Architectural
+Decision Records](https://github.com/tendermint/tendermint/tree/master/docs/architecture).
+
+Learn more by reading the code and comparing it to the
+[specification](https://github.com/tendermint/tendermint/tree/develop/docs/spec).
 
 ## Versioning
 
-### SemVer
+### Semantic Versioning
 
-Tendermint uses [SemVer](http://semver.org/) to determine when and how the version changes.
+Tendermint uses [Semantic Versioning](http://semver.org/) to determine when and how the version changes.
 According to SemVer, anything in the public API can change at any time before version 1.0.0
 
 To provide some stability to Tendermint users in these 0.X.X days, the MINOR version is used
@@ -145,8 +122,40 @@ data into the new chain.
 However, any bump in the PATCH version should be compatible with existing histories
 (if not please open an [issue](https://github.com/tendermint/tendermint/issues)).
 
-For more information on upgrading, see [here](./UPGRADING.md)
+For more information on upgrading, see [UPGRADING.md](./UPGRADING.md)
 
-## Code of Conduct
+## Resources
+
+### Tendermint Core
+
+For details about the blockchain data structures and the p2p protocols, see the
+[Tendermint specification](/docs/spec).
+
+For details on using the software, see the [documentation](/docs/) which is also
+hosted at: https://tendermint.com/docs/
+
+### Tools
+
+Benchmarking and monitoring is provided by `tm-bench` and `tm-monitor`, respectively.
+Their code is found [here](/tools) and these binaries need to be built seperately.
+Additional documentation is found [here](/docs/tools).
+
+### Sub-projects
+
+* [Amino](http://github.com/tendermint/go-amino), reflection-based proto3, with
+  interfaces
+* [IAVL](http://github.com/tendermint/iavl), Merkleized IAVL+ Tree implementation
+
+### Applications
+
+* [Cosmos SDK](http://github.com/cosmos/cosmos-sdk); a cryptocurrency application framework
+* [Ethermint](http://github.com/cosmos/ethermint); Ethereum on Tendermint
+* [Many more](https://tendermint.com/ecosystem)
+
+### Research
+
+* [The latest gossip on BFT consensus](https://arxiv.org/abs/1807.04938)
+* [Master's Thesis on Tendermint](https://atrium.lib.uoguelph.ca/xmlui/handle/10214/9769)
+* [Original Whitepaper](https://tendermint.com/static/docs/tendermint.pdf)
+* [Blog](https://blog.cosmos.network/tendermint/home)
 
-Please read, understand and adhere to our [code of conduct](CODE_OF_CONDUCT.md).

UPGRADING.md

@@ -3,6 +3,55 @@
 This guide provides steps to be followed when you upgrade your applications to
 a newer version of Tendermint Core.
 
+## v0.28.0
+
+This release breaks the format for the `priv_validator.json` file
+and the protocol used for the external validator process.
+It is compatible with v0.27.0 blockchains (neither the BlockProtocol nor the
+P2PProtocol have changed).
+
+Please read carefully for details about upgrading.
+
+**Note:** Backup your `config/priv_validator.json`
+before proceeding.
+
+### `priv_validator.json`
+
+The `config/priv_validator.json` is now two files:
+`config/priv_validator_key.json` and `data/priv_validator_state.json`.
+The former contains the key material, the later contains the details on the last
+message signed.
+
+When running v0.28.0 for the first time, it will back up any pre-existing
+`priv_validator.json` file and proceed to split it into the two new files.
+Upgrading should happen automatically without problem.
+
+To upgrade manually, use the provided `privValUpgrade.go` script, with exact paths for the old
+`priv_validator.json` and the locations for the two new files. It's recomended
+to use the default paths, of `config/priv_validator_key.json` and
+`data/priv_validator_state.json`, respectively:
+
+```
+go run scripts/privValUpgrade.go <old-path> <new-key-path> <new-state-path>
+```
+
+### External validator signers
+
+The Unix and TCP implementations of the remote signing validator
+have been consolidated into a single implementation.
+Thus in both cases, the external process is expected to dial
+Tendermint. This is different from how Unix sockets used to work, where
+Tendermint dialed the external process.
+
+The `PubKeyMsg` was also split into separate `Request` and `Response` types
+for consistency with other messages.
+
+Note that the TCP sockets don't yet use a persistent key,
+so while they're encrypted, they can't yet be properly authenticated.
+See [#3105](https://github.com/tendermint/tendermint/issues/3105).
+Note the Unix socket has neither encryption nor authentication, but will
+add a shared-secret in [#3099](https://github.com/tendermint/tendermint/issues/3099).
+
 ## v0.27.0
 
 This release contains some breaking changes to the block and p2p protocols,

blockchain/store_test.go

@@ -311,7 +311,7 @@ func TestLoadBlockPart(t *testing.T) {
 	gotPart, _, panicErr := doFn(loadPart)
 	require.Nil(t, panicErr, "an existent and proper block should not panic")
 	require.Nil(t, res, "a properly saved block should return a proper block")
-	require.Equal(t, gotPart.(*types.Part).Hash(), part1.Hash(),
+	require.Equal(t, gotPart.(*types.Part), part1,
 		"expecting successful retrieval of previously saved block")
 }
 

cmd/priv_val_server/main.go

@@ -3,6 +3,7 @@ package main
 import (
 	"flag"
 	"os"
+	"time"
 
 	"github.com/tendermint/tendermint/crypto/ed25519"
 	cmn "github.com/tendermint/tendermint/libs/common"
@@ -34,13 +35,20 @@ func main() {
 
 	pv := privval.LoadFilePV(*privValKeyPath, *privValStatePath)
 
-	rs := privval.NewRemoteSigner(
-		logger,
-		*chainID,
-		*addr,
-		pv,
-		ed25519.GenPrivKey(),
-	)
+	var dialer privval.Dialer
+	protocol, address := cmn.ProtocolAndAddress(*addr)
+	switch protocol {
+	case "unix":
+		dialer = privval.DialUnixFn(address)
+	case "tcp":
+		connTimeout := 3 * time.Second // TODO
+		dialer = privval.DialTCPFn(address, connTimeout, ed25519.GenPrivKey())
+	default:
+		logger.Error("Unknown protocol", "protocol", protocol)
+		os.Exit(1)
+	}
+
+	rs := privval.NewRemoteSigner(logger, *chainID, pv, dialer)
 	err := rs.Start()
 	if err != nil {
 		panic(err)

consensus/metrics.go

@@ -8,7 +8,11 @@ import (
 	stdprometheus "github.com/prometheus/client_golang/prometheus"
 )
 
-const MetricsSubsystem = "consensus"
+const (
+	// MetricsSubsystem is a subsystem shared by all metrics exposed by this
+	// package.
+	MetricsSubsystem = "consensus"
+)
 
 // Metrics contains metrics exposed by this package.
 type Metrics struct {
@@ -50,101 +54,107 @@ type Metrics struct {
 }
 
 // PrometheusMetrics returns Metrics build using Prometheus client library.
-func PrometheusMetrics(namespace string) *Metrics {
+// Optionally, labels can be provided along with their values ("foo",
+// "fooValue").
+func PrometheusMetrics(namespace string, labelsAndValues ...string) *Metrics {
+	labels := []string{}
+	for i := 0; i < len(labelsAndValues); i += 2 {
+		labels = append(labels, labelsAndValues[i])
+	}
 	return &Metrics{
 		Height: prometheus.NewGaugeFrom(stdprometheus.GaugeOpts{
 			Namespace: namespace,
 			Subsystem: MetricsSubsystem,
 			Name:      "height",
 			Help:      "Height of the chain.",
-		}, []string{}),
+		}, labels).With(labelsAndValues...),
 		Rounds: prometheus.NewGaugeFrom(stdprometheus.GaugeOpts{
 			Namespace: namespace,
 			Subsystem: MetricsSubsystem,
 			Name:      "rounds",
 			Help:      "Number of rounds.",
-		}, []string{}),
+		}, labels).With(labelsAndValues...),
 
 		Validators: prometheus.NewGaugeFrom(stdprometheus.GaugeOpts{
 			Namespace: namespace,
 			Subsystem: MetricsSubsystem,
 			Name:      "validators",
 			Help:      "Number of validators.",
-		}, []string{}),
+		}, labels).With(labelsAndValues...),
 		ValidatorsPower: prometheus.NewGaugeFrom(stdprometheus.GaugeOpts{
 			Namespace: namespace,
 			Subsystem: MetricsSubsystem,
 			Name:      "validators_power",
 			Help:      "Total power of all validators.",
-		}, []string{}),
+		}, labels).With(labelsAndValues...),
 		MissingValidators: prometheus.NewGaugeFrom(stdprometheus.GaugeOpts{
 			Namespace: namespace,
 			Subsystem: MetricsSubsystem,
 			Name:      "missing_validators",
 			Help:      "Number of validators who did not sign.",
-		}, []string{}),
+		}, labels).With(labelsAndValues...),
 		MissingValidatorsPower: prometheus.NewGaugeFrom(stdprometheus.GaugeOpts{
 			Namespace: namespace,
 			Subsystem: MetricsSubsystem,
 			Name:      "missing_validators_power",
 			Help:      "Total power of the missing validators.",
-		}, []string{}),
+		}, labels).With(labelsAndValues...),
 		ByzantineValidators: prometheus.NewGaugeFrom(stdprometheus.GaugeOpts{
 			Namespace: namespace,
 			Subsystem: MetricsSubsystem,
 			Name:      "byzantine_validators",
 			Help:      "Number of validators who tried to double sign.",
-		}, []string{}),
+		}, labels).With(labelsAndValues...),
 		ByzantineValidatorsPower: prometheus.NewGaugeFrom(stdprometheus.GaugeOpts{
 			Namespace: namespace,
 			Subsystem: MetricsSubsystem,
 			Name:      "byzantine_validators_power",
 			Help:      "Total power of the byzantine validators.",
-		}, []string{}),
+		}, labels).With(labelsAndValues...),
 
 		BlockIntervalSeconds: prometheus.NewGaugeFrom(stdprometheus.GaugeOpts{
 			Namespace: namespace,
 			Subsystem: MetricsSubsystem,
 			Name:      "block_interval_seconds",
 			Help:      "Time between this and the last block.",
-		}, []string{}),
+		}, labels).With(labelsAndValues...),
 
 		NumTxs: prometheus.NewGaugeFrom(stdprometheus.GaugeOpts{
 			Namespace: namespace,
 			Subsystem: MetricsSubsystem,
 			Name:      "num_txs",
 			Help:      "Number of transactions.",
-		}, []string{}),
+		}, labels).With(labelsAndValues...),
 		BlockSizeBytes: prometheus.NewGaugeFrom(stdprometheus.GaugeOpts{
 			Namespace: namespace,
 			Subsystem: MetricsSubsystem,
 			Name:      "block_size_bytes",
 			Help:      "Size of the block.",
-		}, []string{}),
+		}, labels).With(labelsAndValues...),
 		TotalTxs: prometheus.NewGaugeFrom(stdprometheus.GaugeOpts{
 			Namespace: namespace,
 			Subsystem: MetricsSubsystem,
 			Name:      "total_txs",
 			Help:      "Total number of transactions.",
-		}, []string{}),
+		}, labels).With(labelsAndValues...),
 		CommittedHeight: prometheus.NewGaugeFrom(stdprometheus.GaugeOpts{
 			Namespace: namespace,
 			Subsystem: MetricsSubsystem,
 			Name:      "latest_block_height",
 			Help:      "The latest block height.",
-		}, []string{}),
+		}, labels).With(labelsAndValues...),
 		FastSyncing: prometheus.NewGaugeFrom(stdprometheus.GaugeOpts{
 			Namespace: namespace,
 			Subsystem: MetricsSubsystem,
 			Name:      "fast_syncing",
 			Help:      "Whether or not a node is fast syncing. 1 if yes, 0 if no.",
-		}, []string{}),
+		}, labels).With(labelsAndValues...),
 		BlockParts: prometheus.NewCounterFrom(stdprometheus.CounterOpts{
 			Namespace: namespace,
 			Subsystem: MetricsSubsystem,
 			Name:      "block_parts",
 			Help:      "Number of blockparts transmitted by peer.",
-		}, []string{"peer_id"}),
+		}, append(labels, "peer_id")).With(labelsAndValues...),
 	}
 }
 

crypto/ed25519/ed25519.go

@@ -18,8 +18,8 @@ import (
 var _ crypto.PrivKey = PrivKeyEd25519{}
 
 const (
-	PrivKeyAminoRoute = "tendermint/PrivKeyEd25519"
-	PubKeyAminoRoute  = "tendermint/PubKeyEd25519"
+	PrivKeyAminoName = "tendermint/PrivKeyEd25519"
+	PubKeyAminoName  = "tendermint/PubKeyEd25519"
 	// Size of an Edwards25519 signature. Namely the size of a compressed
 	// Edwards25519 point, and a field element. Both of which are 32 bytes.
 	SignatureSize = 64
@@ -30,11 +30,11 @@ var cdc = amino.NewCodec()
 func init() {
 	cdc.RegisterInterface((*crypto.PubKey)(nil), nil)
 	cdc.RegisterConcrete(PubKeyEd25519{},
-		PubKeyAminoRoute, nil)
+		PubKeyAminoName, nil)
 
 	cdc.RegisterInterface((*crypto.PrivKey)(nil), nil)
 	cdc.RegisterConcrete(PrivKeyEd25519{},
-		PrivKeyAminoRoute, nil)
+		PrivKeyAminoName, nil)
 }
 
 // PrivKeyEd25519 implements crypto.PrivKey.

crypto/encoding/amino/amino.go

@@ -12,11 +12,11 @@ import (
 
 var cdc = amino.NewCodec()
 
-// routeTable is used to map public key concrete types back
-// to their amino routes. This should eventually be handled
+// nameTable is used to map public key concrete types back
+// to their registered amino names. This should eventually be handled
 // by amino. Example usage:
-// routeTable[reflect.TypeOf(ed25519.PubKeyEd25519{})] = ed25519.PubKeyAminoRoute
-var routeTable = make(map[reflect.Type]string, 3)
+// nameTable[reflect.TypeOf(ed25519.PubKeyEd25519{})] = ed25519.PubKeyAminoName
+var nameTable = make(map[reflect.Type]string, 3)
 
 func init() {
 	// NOTE: It's important that there be no conflicts here,
@@ -29,16 +29,16 @@ func init() {
 
 	// TODO: Have amino provide a way to go from concrete struct to route directly.
 	// Its currently a private API
-	routeTable[reflect.TypeOf(ed25519.PubKeyEd25519{})] = ed25519.PubKeyAminoRoute
-	routeTable[reflect.TypeOf(secp256k1.PubKeySecp256k1{})] = secp256k1.PubKeyAminoRoute
-	routeTable[reflect.TypeOf(&multisig.PubKeyMultisigThreshold{})] = multisig.PubKeyMultisigThresholdAminoRoute
+	nameTable[reflect.TypeOf(ed25519.PubKeyEd25519{})] = ed25519.PubKeyAminoName
+	nameTable[reflect.TypeOf(secp256k1.PubKeySecp256k1{})] = secp256k1.PubKeyAminoName
+	nameTable[reflect.TypeOf(multisig.PubKeyMultisigThreshold{})] = multisig.PubKeyMultisigThresholdAminoRoute
 }
 
-// PubkeyAminoRoute returns the amino route of a pubkey
+// PubkeyAminoName returns the amino route of a pubkey
 // cdc is currently passed in, as eventually this will not be using
 // a package level codec.
-func PubkeyAminoRoute(cdc *amino.Codec, key crypto.PubKey) (string, bool) {
-	route, found := routeTable[reflect.TypeOf(key)]
+func PubkeyAminoName(cdc *amino.Codec, key crypto.PubKey) (string, bool) {
+	route, found := nameTable[reflect.TypeOf(key)]
 	return route, found
 }
 
@@ -47,17 +47,17 @@ func RegisterAmino(cdc *amino.Codec) {
 	// These are all written here instead of
 	cdc.RegisterInterface((*crypto.PubKey)(nil), nil)
 	cdc.RegisterConcrete(ed25519.PubKeyEd25519{},
-		ed25519.PubKeyAminoRoute, nil)
+		ed25519.PubKeyAminoName, nil)
 	cdc.RegisterConcrete(secp256k1.PubKeySecp256k1{},
-		secp256k1.PubKeyAminoRoute, nil)
+		secp256k1.PubKeyAminoName, nil)
 	cdc.RegisterConcrete(multisig.PubKeyMultisigThreshold{},
 		multisig.PubKeyMultisigThresholdAminoRoute, nil)
 
 	cdc.RegisterInterface((*crypto.PrivKey)(nil), nil)
 	cdc.RegisterConcrete(ed25519.PrivKeyEd25519{},
-		ed25519.PrivKeyAminoRoute, nil)
+		ed25519.PrivKeyAminoName, nil)
 	cdc.RegisterConcrete(secp256k1.PrivKeySecp256k1{},
-		secp256k1.PrivKeyAminoRoute, nil)
+		secp256k1.PrivKeyAminoName, nil)
 }
 
 func PrivKeyFromBytes(privKeyBytes []byte) (privKey crypto.PrivKey, err error) {

crypto/encoding/amino/encode_test.go

@@ -128,18 +128,18 @@ func TestPubKeyInvalidDataProperReturnsEmpty(t *testing.T) {
 	require.Nil(t, pk)
 }
 
-func TestPubkeyAminoRoute(t *testing.T) {
+func TestPubkeyAminoName(t *testing.T) {
 	tests := []struct {
 		key   crypto.PubKey
 		want  string
 		found bool
 	}{
-		{ed25519.PubKeyEd25519{}, ed25519.PubKeyAminoRoute, true},
-		{secp256k1.PubKeySecp256k1{}, secp256k1.PubKeyAminoRoute, true},
-		{&multisig.PubKeyMultisigThreshold{}, multisig.PubKeyMultisigThresholdAminoRoute, true},
+		{ed25519.PubKeyEd25519{}, ed25519.PubKeyAminoName, true},
+		{secp256k1.PubKeySecp256k1{}, secp256k1.PubKeyAminoName, true},
+		{multisig.PubKeyMultisigThreshold{}, multisig.PubKeyMultisigThresholdAminoRoute, true},
 	}
 	for i, tc := range tests {
-		got, found := PubkeyAminoRoute(cdc, tc.key)
+		got, found := PubkeyAminoName(cdc, tc.key)
 		require.Equal(t, tc.found, found, "not equal on tc %d", i)
 		if tc.found {
 			require.Equal(t, tc.want, got, "not equal on tc %d", i)

crypto/merkle/hash.go

@@ -0,0 +1,21 @@
+package merkle
+
+import (
+	"github.com/tendermint/tendermint/crypto/tmhash"
+)
+
+// TODO: make these have a large predefined capacity
+var (
+	leafPrefix  = []byte{0}
+	innerPrefix = []byte{1}
+)
+
+// returns tmhash(0x00 || leaf)
+func leafHash(leaf []byte) []byte {
+	return tmhash.Sum(append(leafPrefix, leaf...))
+}
+
+// returns tmhash(0x01 || left || right)
+func innerHash(left []byte, right []byte) []byte {
+	return tmhash.Sum(append(innerPrefix, append(left, right...)...))
+}

crypto/merkle/proof_simple_value.go

@@ -71,11 +71,11 @@ func (op SimpleValueOp) Run(args [][]byte) ([][]byte, error) {
 	hasher.Write(value) // does not error
 	vhash := hasher.Sum(nil)
 
+	bz := new(bytes.Buffer)
 	// Wrap <op.Key, vhash> to hash the KVPair.
-	hasher = tmhash.New()
-	encodeByteSlice(hasher, []byte(op.key)) // does not error
-	encodeByteSlice(hasher, []byte(vhash))  // does not error
-	kvhash := hasher.Sum(nil)
+	encodeByteSlice(bz, []byte(op.key)) // does not error
+	encodeByteSlice(bz, []byte(vhash))  // does not error
+	kvhash := leafHash(bz.Bytes())
 
 	if !bytes.Equal(kvhash, op.Proof.LeafHash) {
 		return nil, cmn.NewError("leaf hash mismatch: want %X got %X", op.Proof.LeafHash, kvhash)

crypto/merkle/rfc6962_test.go

@@ -0,0 +1,97 @@
+package merkle
+
+// Copyright 2016 Google Inc. All Rights Reserved.
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+//     http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+// These tests were taken from https://github.com/google/trillian/blob/master/merkle/rfc6962/rfc6962_test.go,
+// and consequently fall under the above license.
+import (
+	"bytes"
+	"encoding/hex"
+	"testing"
+
+	"github.com/tendermint/tendermint/crypto/tmhash"
+)
+
+func TestRFC6962Hasher(t *testing.T) {
+	_, leafHashTrail := trailsFromByteSlices([][]byte{[]byte("L123456")})
+	leafHash := leafHashTrail.Hash
+	_, leafHashTrail = trailsFromByteSlices([][]byte{[]byte{}})
+	emptyLeafHash := leafHashTrail.Hash
+	for _, tc := range []struct {
+		desc string
+		got  []byte
+		want string
+	}{
+		// Since creating a merkle tree of no leaves is unsupported here, we skip
+		// the corresponding trillian test vector.
+
+		// Check that the empty hash is not the same as the hash of an empty leaf.
+		// echo -n 00 | xxd -r -p | sha256sum
+		{
+			desc: "RFC6962 Empty Leaf",
+			want: "6e340b9cffb37a989ca544e6bb780a2c78901d3fb33738768511a30617afa01d"[:tmhash.Size*2],
+			got:  emptyLeafHash,
+		},
+		// echo -n 004C313233343536 | xxd -r -p | sha256sum
+		{
+			desc: "RFC6962 Leaf",
+			want: "395aa064aa4c29f7010acfe3f25db9485bbd4b91897b6ad7ad547639252b4d56"[:tmhash.Size*2],
+			got:  leafHash,
+		},
+		// echo -n 014E3132334E343536 | xxd -r -p | sha256sum
+		{
+			desc: "RFC6962 Node",
+			want: "aa217fe888e47007fa15edab33c2b492a722cb106c64667fc2b044444de66bbb"[:tmhash.Size*2],
+			got:  innerHash([]byte("N123"), []byte("N456")),
+		},
+	} {
+		t.Run(tc.desc, func(t *testing.T) {
+			wantBytes, err := hex.DecodeString(tc.want)
+			if err != nil {
+				t.Fatalf("hex.DecodeString(%x): %v", tc.want, err)
+			}
+			if got, want := tc.got, wantBytes; !bytes.Equal(got, want) {
+				t.Errorf("got %x, want %x", got, want)
+			}
+		})
+	}
+}
+
+func TestRFC6962HasherCollisions(t *testing.T) {
+	// Check that different leaves have different hashes.
+	leaf1, leaf2 := []byte("Hello"), []byte("World")
+	_, leafHashTrail := trailsFromByteSlices([][]byte{leaf1})
+	hash1 := leafHashTrail.Hash
+	_, leafHashTrail = trailsFromByteSlices([][]byte{leaf2})
+	hash2 := leafHashTrail.Hash
+	if bytes.Equal(hash1, hash2) {
+		t.Errorf("Leaf hashes should differ, but both are %x", hash1)
+	}
+	// Compute an intermediate subtree hash.
+	_, subHash1Trail := trailsFromByteSlices([][]byte{hash1, hash2})
+	subHash1 := subHash1Trail.Hash
+	// Check that this is not the same as a leaf hash of their concatenation.
+	preimage := append(hash1, hash2...)
+	_, forgedHashTrail := trailsFromByteSlices([][]byte{preimage})
+	forgedHash := forgedHashTrail.Hash
+	if bytes.Equal(subHash1, forgedHash) {
+		t.Errorf("Hasher is not second-preimage resistant")
+	}
+	// Swap the order of nodes and check that the hash is different.
+	_, subHash2Trail := trailsFromByteSlices([][]byte{hash2, hash1})
+	subHash2 := subHash2Trail.Hash
+	if bytes.Equal(subHash1, subHash2) {
+		t.Errorf("Subtree hash does not depend on the order of leaves")
+	}
+}

crypto/merkle/simple_map_test.go

@@ -13,14 +13,14 @@ func TestSimpleMap(t *testing.T) {
 		values []string // each string gets converted to []byte in test
 		want   string
 	}{
-		{[]string{"key1"}, []string{"value1"}, "321d150de16dceb51c72981b432b115045383259b1a550adf8dc80f927508967"},
-		{[]string{"key1"}, []string{"value2"}, "2a9e4baf321eac99f6eecc3406603c14bc5e85bb7b80483cbfc75b3382d24a2f"},
+		{[]string{"key1"}, []string{"value1"}, "a44d3cc7daba1a4600b00a2434b30f8b970652169810d6dfa9fb1793a2189324"},
+		{[]string{"key1"}, []string{"value2"}, "0638e99b3445caec9d95c05e1a3fc1487b4ddec6a952ff337080360b0dcc078c"},
 		// swap order with 2 keys
-		{[]string{"key1", "key2"}, []string{"value1", "value2"}, "c4d8913ab543ba26aa970646d4c99a150fd641298e3367cf68ca45fb45a49881"},
-		{[]string{"key2", "key1"}, []string{"value2", "value1"}, "c4d8913ab543ba26aa970646d4c99a150fd641298e3367cf68ca45fb45a49881"},
+		{[]string{"key1", "key2"}, []string{"value1", "value2"}, "8fd19b19e7bb3f2b3ee0574027d8a5a4cec370464ea2db2fbfa5c7d35bb0cff3"},
+		{[]string{"key2", "key1"}, []string{"value2", "value1"}, "8fd19b19e7bb3f2b3ee0574027d8a5a4cec370464ea2db2fbfa5c7d35bb0cff3"},
 		// swap order with 3 keys
-		{[]string{"key1", "key2", "key3"}, []string{"value1", "value2", "value3"}, "b23cef00eda5af4548a213a43793f2752d8d9013b3f2b64bc0523a4791196268"},
-		{[]string{"key1", "key3", "key2"}, []string{"value1", "value3", "value2"}, "b23cef00eda5af4548a213a43793f2752d8d9013b3f2b64bc0523a4791196268"},
+		{[]string{"key1", "key2", "key3"}, []string{"value1", "value2", "value3"}, "1dd674ec6782a0d586a903c9c63326a41cbe56b3bba33ed6ff5b527af6efb3dc"},
+		{[]string{"key1", "key3", "key2"}, []string{"value1", "value3", "value2"}, "1dd674ec6782a0d586a903c9c63326a41cbe56b3bba33ed6ff5b527af6efb3dc"},
 	}
 	for i, tc := range tests {
 		db := newSimpleMap()

crypto/merkle/simple_proof.go

@@ -5,7 +5,6 @@ import (
 	"errors"
 	"fmt"
 
-	"github.com/tendermint/tendermint/crypto/tmhash"
 	cmn "github.com/tendermint/tendermint/libs/common"
 )
 
@@ -67,7 +66,8 @@ func SimpleProofsFromMap(m map[string][]byte) (rootHash []byte, proofs map[strin
 
 // Verify that the SimpleProof proves the root hash.
 // Check sp.Index/sp.Total manually if needed
-func (sp *SimpleProof) Verify(rootHash []byte, leafHash []byte) error {
+func (sp *SimpleProof) Verify(rootHash []byte, leaf []byte) error {
+	leafHash := leafHash(leaf)
 	if sp.Total < 0 {
 		return errors.New("Proof total must be positive")
 	}
@@ -128,19 +128,19 @@ func computeHashFromAunts(index int, total int, leafHash []byte, innerHashes [][
 		if len(innerHashes) == 0 {
 			return nil
 		}
-		numLeft := (total + 1) / 2
+		numLeft := getSplitPoint(total)
 		if index < numLeft {
 			leftHash := computeHashFromAunts(index, numLeft, leafHash, innerHashes[:len(innerHashes)-1])
 			if leftHash == nil {
 				return nil
 			}
-			return simpleHashFromTwoHashes(leftHash, innerHashes[len(innerHashes)-1])
+			return innerHash(leftHash, innerHashes[len(innerHashes)-1])
 		}
 		rightHash := computeHashFromAunts(index-numLeft, total-numLeft, leafHash, innerHashes[:len(innerHashes)-1])
 		if rightHash == nil {
 			return nil
 		}
-		return simpleHashFromTwoHashes(innerHashes[len(innerHashes)-1], rightHash)
+		return innerHash(innerHashes[len(innerHashes)-1], rightHash)
 	}
 }
 
@@ -182,12 +182,13 @@ func trailsFromByteSlices(items [][]byte) (trails []*SimpleProofNode, root *Simp
 	case 0:
 		return nil, nil
 	case 1:
-		trail := &SimpleProofNode{tmhash.Sum(items[0]), nil, nil, nil}
+		trail := &SimpleProofNode{leafHash(items[0]), nil, nil, nil}
 		return []*SimpleProofNode{trail}, trail
 	default:
-		lefts, leftRoot := trailsFromByteSlices(items[:(len(items)+1)/2])
-		rights, rightRoot := trailsFromByteSlices(items[(len(items)+1)/2:])
-		rootHash := simpleHashFromTwoHashes(leftRoot.Hash, rightRoot.Hash)
+		k := getSplitPoint(len(items))
+		lefts, leftRoot := trailsFromByteSlices(items[:k])
+		rights, rightRoot := trailsFromByteSlices(items[k:])
+		rootHash := innerHash(leftRoot.Hash, rightRoot.Hash)
 		root := &SimpleProofNode{rootHash, nil, nil, nil}
 		leftRoot.Parent = root
 		leftRoot.Right = rightRoot

crypto/merkle/simple_tree.go

@@ -1,23 +1,9 @@
 package merkle
 
 import (
-	"github.com/tendermint/tendermint/crypto/tmhash"
+	"math/bits"
 )
 
-// simpleHashFromTwoHashes is the basic operation of the Merkle tree: Hash(left | right).
-func simpleHashFromTwoHashes(left, right []byte) []byte {
-	var hasher = tmhash.New()
-	err := encodeByteSlice(hasher, left)
-	if err != nil {
-		panic(err)
-	}
-	err = encodeByteSlice(hasher, right)
-	if err != nil {
-		panic(err)
-	}
-	return hasher.Sum(nil)
-}
-
 // SimpleHashFromByteSlices computes a Merkle tree where the leaves are the byte slice,
 // in the provided order.
 func SimpleHashFromByteSlices(items [][]byte) []byte {
@@ -25,11 +11,12 @@ func SimpleHashFromByteSlices(items [][]byte) []byte {
 	case 0:
 		return nil
 	case 1:
-		return tmhash.Sum(items[0])
+		return leafHash(items[0])
 	default:
-		left := SimpleHashFromByteSlices(items[:(len(items)+1)/2])
-		right := SimpleHashFromByteSlices(items[(len(items)+1)/2:])
-		return simpleHashFromTwoHashes(left, right)
+		k := getSplitPoint(len(items))
+		left := SimpleHashFromByteSlices(items[:k])
+		right := SimpleHashFromByteSlices(items[k:])
+		return innerHash(left, right)
 	}
 }
 
@@ -44,3 +31,17 @@ func SimpleHashFromMap(m map[string][]byte) []byte {
 	}
 	return sm.Hash()
 }
+
+// getSplitPoint returns the largest power of 2 less than length
+func getSplitPoint(length int) int {
+	if length < 1 {
+		panic("Trying to split a tree with size < 1")
+	}
+	uLength := uint(length)
+	bitlen := bits.Len(uLength)
+	k := 1 << uint(bitlen-1)
+	if k == length {
+		k >>= 1
+	}
+	return k
+}

crypto/merkle/simple_tree_test.go

@@ -34,7 +34,6 @@ func TestSimpleProof(t *testing.T) {
 
 	// For each item, check the trail.
 	for i, item := range items {
-		itemHash := tmhash.Sum(item)
 		proof := proofs[i]
 
 		// Check total/index
@@ -43,30 +42,53 @@ func TestSimpleProof(t *testing.T) {
 		require.Equal(t, proof.Total, total, "Unmatched totals: %d vs %d", proof.Total, total)
 
 		// Verify success
-		err := proof.Verify(rootHash, itemHash)
-		require.NoError(t, err, "Verificatior failed: %v.", err)
+		err := proof.Verify(rootHash, item)
+		require.NoError(t, err, "Verification failed: %v.", err)
 
 		// Trail too long should make it fail
 		origAunts := proof.Aunts
 		proof.Aunts = append(proof.Aunts, cmn.RandBytes(32))
-		err = proof.Verify(rootHash, itemHash)
+		err = proof.Verify(rootHash, item)
 		require.Error(t, err, "Expected verification to fail for wrong trail length")
 
 		proof.Aunts = origAunts
 
 		// Trail too short should make it fail
 		proof.Aunts = proof.Aunts[0 : len(proof.Aunts)-1]
-		err = proof.Verify(rootHash, itemHash)
+		err = proof.Verify(rootHash, item)
 		require.Error(t, err, "Expected verification to fail for wrong trail length")
 
 		proof.Aunts = origAunts
 
 		// Mutating the itemHash should make it fail.
-		err = proof.Verify(rootHash, MutateByteSlice(itemHash))
+		err = proof.Verify(rootHash, MutateByteSlice(item))
 		require.Error(t, err, "Expected verification to fail for mutated leaf hash")
 
 		// Mutating the rootHash should make it fail.
-		err = proof.Verify(MutateByteSlice(rootHash), itemHash)
+		err = proof.Verify(MutateByteSlice(rootHash), item)
 		require.Error(t, err, "Expected verification to fail for mutated root hash")
 	}
 }
+
+func Test_getSplitPoint(t *testing.T) {
+	tests := []struct {
+		length int
+		want   int
+	}{
+		{1, 0},
+		{2, 1},
+		{3, 2},
+		{4, 2},
+		{5, 4},
+		{10, 8},
+		{20, 16},
+		{100, 64},
+		{255, 128},
+		{256, 128},
+		{257, 256},
+	}
+	for _, tt := range tests {
+		got := getSplitPoint(tt.length)
+		require.Equal(t, tt.want, got, "getSplitPoint(%d) = %v, want %v", tt.length, got, tt.want)
+	}
+}

crypto/multisig/threshold_pubkey.go

@@ -2,7 +2,6 @@ package multisig
 
 import (
 	"github.com/tendermint/tendermint/crypto"
-	"github.com/tendermint/tendermint/crypto/tmhash"
 )
 
 // PubKeyMultisigThreshold implements a K of N threshold multisig.
@@ -11,7 +10,7 @@ type PubKeyMultisigThreshold struct {
 	PubKeys []crypto.PubKey `json:"pubkeys"`
 }
 
-var _ crypto.PubKey = &PubKeyMultisigThreshold{}
+var _ crypto.PubKey = PubKeyMultisigThreshold{}
 
 // NewPubKeyMultisigThreshold returns a new PubKeyMultisigThreshold.
 // Panics if len(pubkeys) < k or 0 >= k.
@@ -22,7 +21,7 @@ func NewPubKeyMultisigThreshold(k int, pubkeys []crypto.PubKey) crypto.PubKey {
 	if len(pubkeys) < k {
 		panic("threshold k of n multisignature: len(pubkeys) < k")
 	}
-	return &PubKeyMultisigThreshold{uint(k), pubkeys}
+	return PubKeyMultisigThreshold{uint(k), pubkeys}
 }
 
 // VerifyBytes expects sig to be an amino encoded version of a MultiSignature.
@@ -31,8 +30,8 @@ func NewPubKeyMultisigThreshold(k int, pubkeys []crypto.PubKey) crypto.PubKey {
 // and all signatures are valid. (Not just k of the signatures)
 // The multisig uses a bitarray, so multiple signatures for the same key is not
 // a concern.
-func (pk *PubKeyMultisigThreshold) VerifyBytes(msg []byte, marshalledSig []byte) bool {
-	var sig *Multisignature
+func (pk PubKeyMultisigThreshold) VerifyBytes(msg []byte, marshalledSig []byte) bool {
+	var sig Multisignature
 	err := cdc.UnmarshalBinaryBare(marshalledSig, &sig)
 	if err != nil {
 		return false
@@ -64,19 +63,19 @@ func (pk *PubKeyMultisigThreshold) VerifyBytes(msg []byte, marshalledSig []byte)
 }
 
 // Bytes returns the amino encoded version of the PubKeyMultisigThreshold
-func (pk *PubKeyMultisigThreshold) Bytes() []byte {
+func (pk PubKeyMultisigThreshold) Bytes() []byte {
 	return cdc.MustMarshalBinaryBare(pk)
 }
 
 // Address returns tmhash(PubKeyMultisigThreshold.Bytes())
-func (pk *PubKeyMultisigThreshold) Address() crypto.Address {
-	return crypto.Address(tmhash.Sum(pk.Bytes()))
+func (pk PubKeyMultisigThreshold) Address() crypto.Address {
+	return crypto.AddressHash(pk.Bytes())
 }
 
 // Equals returns true iff pk and other both have the same number of keys, and
 // all constituent keys are the same, and in the same order.
-func (pk *PubKeyMultisigThreshold) Equals(other crypto.PubKey) bool {
-	otherKey, sameType := other.(*PubKeyMultisigThreshold)
+func (pk PubKeyMultisigThreshold) Equals(other crypto.PubKey) bool {
+	otherKey, sameType := other.(PubKeyMultisigThreshold)
 	if !sameType {
 		return false
 	}

crypto/multisig/threshold_pubkey_test.go

@@ -82,7 +82,7 @@ func TestMultiSigPubKeyEquality(t *testing.T) {
 	msg := []byte{1, 2, 3, 4}
 	pubkeys, _ := generatePubKeysAndSignatures(5, msg)
 	multisigKey := NewPubKeyMultisigThreshold(2, pubkeys)
-	var unmarshalledMultisig *PubKeyMultisigThreshold
+	var unmarshalledMultisig PubKeyMultisigThreshold
 	cdc.MustUnmarshalBinaryBare(multisigKey.Bytes(), &unmarshalledMultisig)
 	require.True(t, multisigKey.Equals(unmarshalledMultisig))
 
@@ -95,6 +95,13 @@ func TestMultiSigPubKeyEquality(t *testing.T) {
 	require.False(t, multisigKey.Equals(multisigKey2))
 }
 
+func TestAddress(t *testing.T) {
+	msg := []byte{1, 2, 3, 4}
+	pubkeys, _ := generatePubKeysAndSignatures(5, msg)
+	multisigKey := NewPubKeyMultisigThreshold(2, pubkeys)
+	require.Len(t, multisigKey.Address().Bytes(), 20)
+}
+
 func TestPubKeyMultisigThresholdAminoToIface(t *testing.T) {
 	msg := []byte{1, 2, 3, 4}
 	pubkeys, _ := generatePubKeysAndSignatures(5, msg)

crypto/multisig/wire.go

@@ -20,7 +20,7 @@ func init() {
 	cdc.RegisterConcrete(PubKeyMultisigThreshold{},
 		PubKeyMultisigThresholdAminoRoute, nil)
 	cdc.RegisterConcrete(ed25519.PubKeyEd25519{},
-		ed25519.PubKeyAminoRoute, nil)
+		ed25519.PubKeyAminoName, nil)
 	cdc.RegisterConcrete(secp256k1.PubKeySecp256k1{},
-		secp256k1.PubKeyAminoRoute, nil)
+		secp256k1.PubKeyAminoName, nil)
 }

crypto/secp256k1/secp256k1.go

@@ -16,8 +16,8 @@ import (
 
 //-------------------------------------
 const (
-	PrivKeyAminoRoute = "tendermint/PrivKeySecp256k1"
-	PubKeyAminoRoute  = "tendermint/PubKeySecp256k1"
+	PrivKeyAminoName = "tendermint/PrivKeySecp256k1"
+	PubKeyAminoName  = "tendermint/PubKeySecp256k1"
 )
 
 var cdc = amino.NewCodec()
@@ -25,11 +25,11 @@ var cdc = amino.NewCodec()
 func init() {
 	cdc.RegisterInterface((*crypto.PubKey)(nil), nil)
 	cdc.RegisterConcrete(PubKeySecp256k1{},
-		PubKeyAminoRoute, nil)
+		PubKeyAminoName, nil)
 
 	cdc.RegisterInterface((*crypto.PrivKey)(nil), nil)
 	cdc.RegisterConcrete(PrivKeySecp256k1{},
-		PrivKeyAminoRoute, nil)
+		PrivKeyAminoName, nil)
 }
 
 //-------------------------------------

docs/app-dev/app-architecture.md

@@ -45,6 +45,6 @@ Tendermint.
 See the following for more extensive documentation:
 
 - [Interchain Standard for the Light-Client REST API](https://github.com/cosmos/cosmos-sdk/pull/1028)
-- [Tendermint RPC Docs](https://tendermint.github.io/slate/)
+- [Tendermint RPC Docs](https://tendermint.com/rpc/)
 - [Tendermint in Production](../tendermint-core/running-in-production.md)
 - [ABCI spec](./abci-spec.md)

docs/app-dev/ecosystem.json

@@ -63,6 +63,13 @@
       "author": "Zach Balder",
       "description": "Public service reporting and tracking"
     },
+    {
+      "name": "ParadigmCore",
+      "url": "https://github.com/ParadigmFoundation/ParadigmCore",
+      "language": "TypeScript",
+      "author": "Paradigm Labs",
+      "description": "Reference implementation of the Paradigm Protocol, and OrderStream network client."
+    },
     {
       "name": "Passchain",
       "url": "https://github.com/trusch/passchain",

docs/app-dev/indexing-transactions.md

@@ -78,7 +78,7 @@ endpoint:
 curl "localhost:26657/tx_search?query=\"account.name='igor'\"&prove=true"
 ```
 
-Check out [API docs](https://tendermint.github.io/slate/?shell#txsearch)
+Check out [API docs](https://tendermint.com/rpc/#txsearch)
 for more information on query syntax and other options.
 
 ## Subscribing to transactions
@@ -97,5 +97,5 @@ by providing a query to `/subscribe` RPC endpoint.
 }
 ```
 
-Check out [API docs](https://tendermint.github.io/slate/#subscribe) for
+Check out [API docs](https://tendermint.com/rpc/#subscribe) for
 more information on query syntax and other options.

docs/architecture/adr-020-block-size.md

@@ -7,6 +7,7 @@
 28-08-2018: Third version after Ethan's comments
 30-08-2018: AminoOverheadForBlock => MaxAminoOverheadForBlock
 31-08-2018: Bounding evidence and chain ID
+13-01-2019: Add section on MaxBytes vs MaxDataBytes
 
 ## Context
 
@@ -20,6 +21,32 @@ We should just remove MaxTxs all together and stick with MaxBytes, and have a
 But we can't just reap BlockSize.MaxBytes, since MaxBytes is for the entire block,
 not for the txs inside the block. There's extra amino overhead + the actual
 headers on top of the actual transactions + evidence + last commit.
+We could also consider using a MaxDataBytes instead of or in addition to MaxBytes.
+
+## MaxBytes vs MaxDataBytes
+
+The [PR #3045](https://github.com/tendermint/tendermint/pull/3045) suggested
+additional clarity/justification was necessary here, wither respect to the use
+of MaxDataBytes in addition to, or instead of, MaxBytes.
+
+MaxBytes provides a clear limit on the total size of a block that requires no
+additional calculation if you want to use it to bound resource usage, and there
+has been considerable discussions about optimizing tendermint around 1MB blocks.
+Regardless, we need some maximum on the size of a block so we can avoid
+unmarshalling blocks that are too big during the consensus, and it seems more
+straightforward to provide a single fixed number for this rather than a
+computation of "MaxDataBytes + everything else you need to make room for
+(signatures, evidence, header)". MaxBytes provides a simple bound so we can
+always say "blocks are less than X MB".
+
+Having both MaxBytes and MaxDataBytes feels like unnecessary complexity. It's
+not particularly surprising for MaxBytes to imply the maximum size of the
+entire block (not just txs), one just has to know that a block includes header,
+txs, evidence, votes. For more fine grained control over the txs included in the
+block, there is the MaxGas. In practice, the MaxGas may be expected to do most of
+the tx throttling, and the MaxBytes to just serve as an upper bound on the total
+size. Applications can use MaxGas as a MaxDataBytes by just taking the gas for
+every tx to be its size in bytes.
 
 ## Proposed solution
 
@@ -61,7 +88,7 @@ MaxXXX stayed the same.
 
 ## Status
 
-Proposed.
+Accepted.
 
 ## Consequences
 

docs/architecture/adr-030-consensus-refactor.md

@@ -126,6 +126,312 @@ func TestConsensusXXX(t *testing.T) {
 }
 ```
 
+
+## Consensus Executor
+
+## Consensus Core
+
+```go
+type Event interface{}
+
+type EventNewHeight struct {
+    Height           int64
+    ValidatorId      int
+}
+
+type EventNewRound HeightAndRound
+
+type EventProposal struct {
+    Height           int64
+    Round            int
+    Timestamp        Time
+    BlockID          BlockID
+    POLRound         int
+    Sender           int   
+}
+
+type Majority23PrevotesBlock struct {
+    Height           int64
+    Round            int
+    BlockID          BlockID
+}
+
+type Majority23PrecommitBlock struct {
+    Height           int64
+    Round            int
+    BlockID          BlockID
+}
+
+type HeightAndRound struct {
+    Height           int64
+    Round            int
+}
+
+type Majority23PrevotesAny HeightAndRound
+type Majority23PrecommitAny HeightAndRound
+type TimeoutPropose HeightAndRound
+type TimeoutPrevotes HeightAndRound
+type TimeoutPrecommit HeightAndRound
+
+
+type Message interface{}
+
+type MessageProposal struct {
+    Height           int64
+    Round            int
+    BlockID          BlockID
+    POLRound         int
+}
+
+type VoteType int
+
+const (
+	VoteTypeUnknown VoteType = iota
+	Prevote
+	Precommit
+)
+
+
+type MessageVote struct {
+    Height           int64
+    Round            int
+    BlockID          BlockID
+    Type             VoteType
+}
+
+
+type MessageDecision struct {
+    Height           int64
+    Round            int
+    BlockID          BlockID
+}
+
+type TriggerTimeout struct {
+    Height           int64
+    Round            int
+    Duration         Duration
+}
+
+
+type RoundStep int
+
+const (
+	RoundStepUnknown RoundStep = iota
+	RoundStepPropose       
+	RoundStepPrevote
+	RoundStepPrecommit
+	RoundStepCommit
+)
+
+type State struct {
+	Height           int64
+	Round            int
+	Step             RoundStep
+	LockedValue      BlockID
+	LockedRound      int
+	ValidValue       BlockID
+	ValidRound       int
+	ValidatorId      int
+	ValidatorSetSize int
+}
+
+func proposer(height int64, round int) int {}
+func getValue() BlockID {}
+
+func Consensus(event Event, state State) (State, Message, TriggerTimeout) {
+    msg = nil
+    timeout = nil
+	switch event := event.(type) {
+    	case EventNewHeight:
+    		if event.Height > state.Height {
+    		    state.Height = event.Height
+    		    state.Round = -1
+    		    state.Step = RoundStepPropose
+    		    state.LockedValue = nil
+    		    state.LockedRound = -1
+    		    state.ValidValue = nil
+    		    state.ValidRound = -1
+    		    state.ValidatorId = event.ValidatorId
+    		} 
+    	    return state, msg, timeout
+    	
+    	case EventNewRound:
+    		if event.Height == state.Height and event.Round > state.Round {
+               state.Round = eventRound
+               state.Step = RoundStepPropose
+               if proposer(state.Height, state.Round) == state.ValidatorId {
+                   proposal = state.ValidValue
+                   if proposal == nil {
+                   	    proposal = getValue()
+                   }
+                   msg =  MessageProposal { state.Height, state.Round, proposal, state.ValidRound }
+               }
+               timeout = TriggerTimeout { state.Height, state.Round, timeoutPropose(state.Round) }
+            }
+    	    return state, msg, timeout
+    	
+    	case EventProposal:
+    		if event.Height == state.Height and event.Round == state.Round and 
+    	       event.Sender == proposal(state.Height, state.Round) and state.Step == RoundStepPropose { 
+    	       	if event.POLRound >= state.LockedRound or event.BlockID == state.BlockID or state.LockedRound == -1 {
+    	       		msg = MessageVote { state.Height, state.Round, event.BlockID, Prevote }
+    	       	}
+    	       	state.Step = RoundStepPrevote
+            }
+    	    return state, msg, timeout
+    	
+    	case TimeoutPropose:
+    		if event.Height == state.Height and event.Round == state.Round and state.Step == RoundStepPropose {
+    		    msg = MessageVote { state.Height, state.Round, nil, Prevote }
+    			state.Step = RoundStepPrevote
+            }
+    	    return state, msg, timeout
+    	
+    	case Majority23PrevotesBlock:
+    		if event.Height == state.Height and event.Round == state.Round and state.Step >= RoundStepPrevote and event.Round > state.ValidRound {
+    		    state.ValidRound = event.Round
+    		    state.ValidValue = event.BlockID
+    		    if state.Step == RoundStepPrevote {
+    		    	state.LockedRound = event.Round
+    		    	state.LockedValue = event.BlockID
+    		    	msg = MessageVote { state.Height, state.Round, event.BlockID, Precommit }
+    		    	state.Step = RoundStepPrecommit
+    		    }
+            }
+    	    return state, msg, timeout
+    	
+    	case Majority23PrevotesAny:
+    		if event.Height == state.Height and event.Round == state.Round and state.Step == RoundStepPrevote {
+    			timeout = TriggerTimeout { state.Height, state.Round, timeoutPrevote(state.Round) }
+    		}
+    	    return state, msg, timeout
+    	
+    	case TimeoutPrevote:
+    		if event.Height == state.Height and event.Round == state.Round and state.Step == RoundStepPrevote {
+    			msg = MessageVote { state.Height, state.Round, nil, Precommit }
+    			state.Step = RoundStepPrecommit
+    		}
+    	    return state, msg, timeout
+    	
+    	case Majority23PrecommitBlock:
+    		if event.Height == state.Height {
+    		    state.Step = RoundStepCommit
+    		    state.LockedValue = event.BlockID
+    		}
+    	    return state, msg, timeout
+    		
+    	case Majority23PrecommitAny:
+    		if event.Height == state.Height and event.Round == state.Round {
+    			timeout = TriggerTimeout { state.Height, state.Round, timeoutPrecommit(state.Round) }
+    		}
+    	    return state, msg, timeout
+    	
+    	case TimeoutPrecommit:
+            if event.Height == state.Height and event.Round == state.Round {
+            	state.Round = state.Round + 1
+            }
+    	    return state, msg, timeout
+	}
+}	
+
+func ConsensusExecutor() {
+	proposal = nil
+	votes = HeightVoteSet { Height: 1 }
+	state = State {
+		Height:       1
+		Round:        0          
+		Step:         RoundStepPropose
+		LockedValue:  nil
+		LockedRound:  -1
+		ValidValue:   nil
+		ValidRound:   -1
+	}
+	
+	event = EventNewHeight {1, id}
+	state, msg, timeout = Consensus(event, state)
+	
+	event = EventNewRound {state.Height, 0}
+	state, msg, timeout = Consensus(event, state)
+	
+	if msg != nil {
+		send msg
+	}
+	
+	if timeout != nil {
+		trigger timeout
+	}
+	
+	for {
+		select {
+		    case message := <- msgCh:
+		    	switch msg := message.(type) {
+		    	    case MessageProposal:
+		    	        
+		    	    case MessageVote:	
+		    	    	if msg.Height == state.Height {
+		    	    		newVote = votes.AddVote(msg)
+		    	    		if newVote {
+		    	    			switch msg.Type {
+                                	case Prevote:
+                                		prevotes = votes.Prevotes(msg.Round)
+                                		if prevotes.WeakCertificate() and msg.Round > state.Round {
+                                			event = EventNewRound { msg.Height, msg.Round }
+                                			state, msg, timeout = Consensus(event, state)
+                                			state = handleStateChange(state, msg, timeout)
+                                		}	
+                                		
+                                		if blockID, ok = prevotes.TwoThirdsMajority(); ok and blockID != nil {
+                                		    if msg.Round == state.Round and hasBlock(blockID) {
+                                		    	event = Majority23PrevotesBlock { msg.Height, msg.Round, blockID }
+                                		    	state, msg, timeout = Consensus(event, state)
+                                		    	state = handleStateChange(state, msg, timeout)
+                                		    } 
+                                		    if proposal != nil and proposal.POLRound == msg.Round and hasBlock(blockID) {
+                                		        event = EventProposal {
+                                                        Height: state.Height
+                                                        Round:  state.Round
+                                                        BlockID: blockID
+                                                        POLRound: proposal.POLRound
+                                                        Sender: message.Sender
+                                		        }
+                                		        state, msg, timeout = Consensus(event, state)
+                                		        state = handleStateChange(state, msg, timeout)
+                                		    }
+                                		}
+                                		
+                                		if prevotes.HasTwoThirdsAny() and msg.Round == state.Round {
+                                			event = Majority23PrevotesAny { msg.Height, msg.Round, blockID }
+                                			state, msg, timeout = Consensus(event, state)
+                                            state = handleStateChange(state, msg, timeout)
+                                		}
+                                		
+                                	case Precommit:	
+                                		
+		    	    		    }
+		    	    	    }
+		    	        }
+		    case timeout := <- timeoutCh:
+		    
+		    case block := <- blockCh:	
+		    	
+		}
+	}
+}
+	
+func handleStateChange(state, msg, timeout) State {
+	if state.Step == Commit {
+		state = ExecuteBlock(state.LockedValue)
+	}	
+	if msg != nil {
+		send msg
+	}	
+	if timeout != nil {
+		trigger timeout
+	}	
+}
+
+```
+
 ### Implementation roadmap
 
 * implement proposed implementation

docs/networks/docker-compose.md

@@ -4,7 +4,7 @@ With Docker Compose, you can spin up local testnets with a single command.
 
 ## Requirements
 
-1. [Install tendermint](/docs/install.md)
+1. [Install tendermint](/docs/introduction/install.md)
 2. [Install docker](https://docs.docker.com/engine/installation/)
 3. [Install docker-compose](https://docs.docker.com/compose/install/)
 

docs/spec/blockchain/blockchain.md

@@ -109,10 +109,6 @@ Tendermint uses the
 [Google.Protobuf.WellKnownTypes.Timestamp](https://developers.google.com/protocol-buffers/docs/reference/csharp/class/google/protobuf/well-known-types/timestamp)
 format, which uses two integers, one for Seconds and for Nanoseconds.
 
-NOTE: there is currently a small divergence between Tendermint and the
-Google.Protobuf.WellKnownTypes.Timestamp that should be resolved. See [this
-issue](https://github.com/tendermint/go-amino/issues/223) for details.
-
 ## Data
 
 Data is just a wrapper for a list of transactions, where transactions are

docs/spec/blockchain/encoding.md

@@ -144,12 +144,17 @@ func MakeParts(obj interface{}, partSize int) []Part
 For an overview of Merkle trees, see
 [wikipedia](https://en.wikipedia.org/wiki/Merkle_tree)
 
-A Simple Tree is a simple compact binary tree for a static list of items. Simple Merkle trees are used in numerous places in Tendermint to compute a cryptographic digest of a data structure. In a Simple Tree, the transactions and validation signatures of a block are hashed using this simple merkle tree logic.
+We use the RFC 6962 specification of a merkle tree, instantiated with sha256 as the hash function.
+Merkle trees are used throughout Tendermint to compute a cryptographic digest of a data structure.
+The differences between RFC 6962 and the simplest form a merkle tree are that:
 
-If the number of items is not a power of two, the tree will not be full
-and some leaf nodes will be at different levels. Simple Tree tries to
-keep both sides of the tree the same size, but the left side may be one
-greater, for example:
+1) leaf nodes and inner nodes have different hashes. 
+   This is to prevent a proof to an inner node, claiming that it is the hash of the leaf. 
+   The leaf nodes are `SHA256(0x00 || leaf_data)`, and inner nodes are `SHA256(0x01 || left_hash || right_hash)`.
+
+2) When the number of items isn't a power of two, the left half of the tree is as big as it could be.
+   (The smallest power of two less than the number of items) This allows new leaves to be added with less
+   recomputation. For example:
 
 ```
    Simple Tree with 6 items           Simple Tree with 7 items
@@ -163,48 +168,31 @@ greater, for example:
      / \             / \                / \             / \
     /   \           /   \              /   \           /   \
    /     \         /     \            /     \         /     \
-  *       h2      *       h5         *       *       *       h6
- / \             / \                / \     / \     / \
-h0  h1          h3  h4             h0  h1  h2  h3  h4  h5
-```
-
-Tendermint always uses the `TMHASH` hash function, which is equivalent to
-SHA256:
-
-```
-func TMHASH(bz []byte) []byte {
-    return SHA256(bz)
-}
+  *       *       h4     h5          *       *       *       h6
+ / \     / \                        / \     / \     / \
+h0  h1  h2 h3                      h0  h1  h2  h3  h4  h5
 ```
 
 ### Simple Merkle Root
 
-The function `SimpleMerkleRoot` is a simple recursive function defined as follows:
+The function `MerkleRoot` is a simple recursive function defined as follows:
 
 ```go
-func SimpleMerkleRoot(hashes [][]byte) []byte{
-    switch len(hashes) {
-    case 0:
-        return nil
-    case 1:
-        return hashes[0]
-    default:
-        left := SimpleMerkleRoot(hashes[:(len(hashes)+1)/2])
-        right := SimpleMerkleRoot(hashes[(len(hashes)+1)/2:])
-        return SimpleConcatHash(left, right)
-    }
-}
-
-func SimpleConcatHash(left, right []byte) []byte{
-    left = encodeByteSlice(left)
-    right = encodeByteSlice(right)
-    return TMHASH(append(left, right))
+func MerkleRootFromLeafs(leafs [][]byte) []byte{
+	switch len(items) {
+	case 0:
+		return nil
+	case 1:
+		return leafHash(leafs[0])      // SHA256(0x00 || leafs[0])
+	default:
+		k := getSplitPoint(len(items)) // largest power of two smaller than items
+		left := MerkleRootFromLeafs(items[:k])
+		right := MerkleRootFromLeafs(items[k:])
+		return innerHash(left, right)  // SHA256(0x01 || left || right)
+	}
 }
 ```
 
-Note that the leaves are Amino encoded as byte-arrays (ie. simple Uvarint length
-prefix) before being concatenated together and hashed.
-
 Note: we will abuse notion and invoke `SimpleMerkleRoot` with arguments of type `struct` or type `[]struct`.
 For `struct` arguments, we compute a `[][]byte` containing the hash of each
 field in the struct, in the same order the fields appear in the struct.
@@ -214,7 +202,7 @@ For `[]struct` arguments, we compute a `[][]byte` by hashing the individual `str
 
 Proof that a leaf is in a Merkle tree consists of a simple structure:
 
-```
+```golang
 type SimpleProof struct {
         Aunts [][]byte
 }
@@ -222,7 +210,7 @@ type SimpleProof struct {
 
 Which is verified using the following:
 
-```
+```golang
 func (proof SimpleProof) Verify(index, total int, leafHash, rootHash []byte) bool {
 	computedHash := computeHashFromAunts(index, total, leafHash, proof.Aunts)
     return computedHash == rootHash
@@ -238,7 +226,7 @@ func computeHashFromAunts(index, total int, leafHash []byte, innerHashes [][]byt
 
 	assert(len(innerHashes) > 0)
 
-	numLeft := (total + 1) / 2
+	numLeft := getSplitPoint(total) // largest power of 2 less than total
 	if index < numLeft {
 		leftHash := computeHashFromAunts(index, numLeft, leafHash, innerHashes[:len(innerHashes)-1])
 		assert(leftHash != nil)
@@ -301,8 +289,8 @@ type CanonicalVote struct {
 	Type      byte
 	Height    int64            `binary:"fixed64"`
 	Round     int64            `binary:"fixed64"`
-	Timestamp time.Time
 	BlockID   CanonicalBlockID
+	Timestamp time.Time
 	ChainID   string
 }
 ```

docs/spec/consensus/signing.md

@@ -59,9 +59,9 @@ type PartSetHeader struct {
 ```
 
 To be included in a valid vote or proposal, BlockID must either represent a `nil` block, or a complete one.
-We introduce two methods, `BlockID.IsNil()` and `BlockID.IsComplete()` for these cases, respectively.
+We introduce two methods, `BlockID.IsZero()` and `BlockID.IsComplete()` for these cases, respectively.
 
-`BlockID.IsNil()` returns true for BlockID `b` if each of the following
+`BlockID.IsZero()` returns true for BlockID `b` if each of the following
 are true:
 
 ```
@@ -81,7 +81,7 @@ len(b.PartsHeader.Hash) == 32
 
 ## Proposals
 
-The structure of a propsal for signing looks like:
+The structure of a proposal for signing looks like:
 
 ```
 type CanonicalProposal struct {
@@ -118,8 +118,8 @@ type CanonicalVote struct {
 	Type      SignedMsgType // type alias for byte
 	Height    int64         `binary:"fixed64"`
 	Round     int64         `binary:"fixed64"`
-	Timestamp time.Time
 	BlockID   BlockID
+	Timestamp time.Time
 	ChainID   string
 }
 ```
@@ -130,7 +130,7 @@ A vote is valid if each of the following lines evaluates to true for vote `v`:
 v.Type == 0x1 || v.Type == 0x2
 v.Height > 0
 v.Round >= 0
-v.BlockID.IsNil() || v.BlockID.IsValid()
+v.BlockID.IsZero() || v.BlockID.IsComplete()
 ```
 
 In other words, a vote is valid for signing if it contains the type of a Prevote

libs/log/tmfmt_logger.go

@@ -90,7 +90,7 @@ func (l tmfmtLogger) Log(keyvals ...interface{}) error {
 	//     D										- first character of the level, uppercase (ASCII only)
 	//     [2016-05-02|11:06:44.322]    - our time format (see https://golang.org/src/time/format.go)
 	//     Stopping ...					- message
-	enc.buf.WriteString(fmt.Sprintf("%c[%s] %-44s ", lvl[0]-32, time.Now().Format("2016-01-02|15:04:05.000"), msg))
+	enc.buf.WriteString(fmt.Sprintf("%c[%s] %-44s ", lvl[0]-32, time.Now().Format("2006-01-02|15:04:05.000"), msg))
 
 	if module != unknown {
 		enc.buf.WriteString("module=" + module + " ")

lite/proxy/query_test.go

@@ -10,6 +10,7 @@ import (
 	"github.com/stretchr/testify/require"
 
 	"github.com/tendermint/tendermint/abci/example/kvstore"
+	"github.com/tendermint/tendermint/crypto/merkle"
 	"github.com/tendermint/tendermint/lite"
 	certclient "github.com/tendermint/tendermint/lite/client"
 	nm "github.com/tendermint/tendermint/node"
@@ -143,12 +144,13 @@ func TestTxProofs(t *testing.T) {
 	require.NotNil(err)
 	require.Contains(err.Error(), "not found")
 
-	// Now let's check with the real tx hash.
+	// Now let's check with the real tx root hash.
 	key = types.Tx(tx).Hash()
 	res, err = cl.Tx(key, true)
 	require.NoError(err, "%#v", err)
 	require.NotNil(res)
-	err = res.Proof.Validate(key)
+	keyHash := merkle.SimpleHashFromByteSlices([][]byte{key})
+	err = res.Proof.Validate(keyHash)
 	assert.NoError(err, "%#v", err)
 
 	commit, err := GetCertifiedCommit(br.Height, cl, cert)

mempool/metrics.go

@@ -7,7 +7,11 @@ import (
 	stdprometheus "github.com/prometheus/client_golang/prometheus"
 )
 
-const MetricsSubsytem = "mempool"
+const (
+	// MetricsSubsystem is a subsystem shared by all metrics exposed by this
+	// package.
+	MetricsSubsystem = "mempool"
+)
 
 // Metrics contains metrics exposed by this package.
 // see MetricsProvider for descriptions.
@@ -23,33 +27,39 @@ type Metrics struct {
 }
 
 // PrometheusMetrics returns Metrics build using Prometheus client library.
-func PrometheusMetrics(namespace string) *Metrics {
+// Optionally, labels can be provided along with their values ("foo",
+// "fooValue").
+func PrometheusMetrics(namespace string, labelsAndValues ...string) *Metrics {
+	labels := []string{}
+	for i := 0; i < len(labelsAndValues); i += 2 {
+		labels = append(labels, labelsAndValues[i])
+	}
 	return &Metrics{
 		Size: prometheus.NewGaugeFrom(stdprometheus.GaugeOpts{
 			Namespace: namespace,
-			Subsystem: MetricsSubsytem,
+			Subsystem: MetricsSubsystem,
 			Name:      "size",
 			Help:      "Size of the mempool (number of uncommitted transactions).",
-		}, []string{}),
+		}, labels).With(labelsAndValues...),
 		TxSizeBytes: prometheus.NewHistogramFrom(stdprometheus.HistogramOpts{
 			Namespace: namespace,
-			Subsystem: MetricsSubsytem,
+			Subsystem: MetricsSubsystem,
 			Name:      "tx_size_bytes",
 			Help:      "Transaction sizes in bytes.",
 			Buckets:   stdprometheus.ExponentialBuckets(1, 3, 17),
-		}, []string{}),
+		}, labels).With(labelsAndValues...),
 		FailedTxs: prometheus.NewCounterFrom(stdprometheus.CounterOpts{
 			Namespace: namespace,
-			Subsystem: MetricsSubsytem,
+			Subsystem: MetricsSubsystem,
 			Name:      "failed_txs",
 			Help:      "Number of failed transactions.",
-		}, []string{}),
+		}, labels).With(labelsAndValues...),
 		RecheckTimes: prometheus.NewCounterFrom(stdprometheus.CounterOpts{
 			Namespace: namespace,
-			Subsystem: MetricsSubsytem,
+			Subsystem: MetricsSubsystem,
 			Name:      "recheck_times",
 			Help:      "Number of times transactions are rechecked in the mempool.",
-		}, []string{}),
+		}, labels).With(labelsAndValues...),
 	}
 }
 

node/node.go

@@ -117,15 +117,17 @@ func DefaultNewNode(config *cfg.Config, logger log.Logger) (*Node, error) {
 }
 
 // MetricsProvider returns a consensus, p2p and mempool Metrics.
-type MetricsProvider func() (*cs.Metrics, *p2p.Metrics, *mempl.Metrics, *sm.Metrics)
+type MetricsProvider func(chainID string) (*cs.Metrics, *p2p.Metrics, *mempl.Metrics, *sm.Metrics)
 
 // DefaultMetricsProvider returns Metrics build using Prometheus client library
 // if Prometheus is enabled. Otherwise, it returns no-op Metrics.
 func DefaultMetricsProvider(config *cfg.InstrumentationConfig) MetricsProvider {
-	return func() (*cs.Metrics, *p2p.Metrics, *mempl.Metrics, *sm.Metrics) {
+	return func(chainID string) (*cs.Metrics, *p2p.Metrics, *mempl.Metrics, *sm.Metrics) {
 		if config.Prometheus {
-			return cs.PrometheusMetrics(config.Namespace), p2p.PrometheusMetrics(config.Namespace),
-				mempl.PrometheusMetrics(config.Namespace), sm.PrometheusMetrics(config.Namespace)
+			return cs.PrometheusMetrics(config.Namespace, "chain_id", chainID),
+				p2p.PrometheusMetrics(config.Namespace, "chain_id", chainID),
+				mempl.PrometheusMetrics(config.Namespace, "chain_id", chainID),
+				sm.PrometheusMetrics(config.Namespace, "chain_id", chainID)
 		}
 		return cs.NopMetrics(), p2p.NopMetrics(), mempl.NopMetrics(), sm.NopMetrics()
 	}
@@ -274,7 +276,7 @@ func NewNode(config *cfg.Config,
 		consensusLogger.Info("This node is not a validator", "addr", addr, "pubKey", pubKey)
 	}
 
-	csMetrics, p2pMetrics, memplMetrics, smMetrics := metricsProvider()
+	csMetrics, p2pMetrics, memplMetrics, smMetrics := metricsProvider(genDoc.ChainID)
 
 	// Make MempoolReactor
 	mempool := mempl.NewMempool(
@@ -878,16 +880,20 @@ func createAndStartPrivValidatorSocketClient(
 	listenAddr string,
 	logger log.Logger,
 ) (types.PrivValidator, error) {
-	var pvsc types.PrivValidator
+	var listener net.Listener
 
 	protocol, address := cmn.ProtocolAndAddress(listenAddr)
+	ln, err := net.Listen(protocol, address)
+	if err != nil {
+		return nil, err
+	}
 	switch protocol {
 	case "unix":
-		pvsc = privval.NewIPCVal(logger.With("module", "privval"), address)
+		listener = privval.NewUnixListener(ln)
 	case "tcp":
 		// TODO: persist this key so external signer
 		// can actually authenticate us
-		pvsc = privval.NewTCPVal(logger.With("module", "privval"), listenAddr, ed25519.GenPrivKey())
+		listener = privval.NewTCPListener(ln, ed25519.GenPrivKey())
 	default:
 		return nil, fmt.Errorf(
 			"Wrong listen address: expected either 'tcp' or 'unix' protocols, got %s",
@@ -895,10 +901,9 @@ func createAndStartPrivValidatorSocketClient(
 		)
 	}
 
-	if pvsc, ok := pvsc.(cmn.Service); ok {
-		if err := pvsc.Start(); err != nil {
-			return nil, errors.Wrap(err, "failed to start")
-		}
+	pvsc := privval.NewSocketVal(logger.With("module", "privval"), listener)
+	if err := pvsc.Start(); err != nil {
+		return nil, errors.Wrap(err, "failed to start private validator")
 	}
 
 	return pvsc, nil

node/node_test.go

@@ -122,25 +122,25 @@ func TestNodeSetPrivValTCP(t *testing.T) {
 	config := cfg.ResetTestRoot("node_priv_val_tcp_test")
 	config.BaseConfig.PrivValidatorListenAddr = addr
 
-	rs := privval.NewRemoteSigner(
+	dialer := privval.DialTCPFn(addr, 100*time.Millisecond, ed25519.GenPrivKey())
+	pvsc := privval.NewRemoteSigner(
 		log.TestingLogger(),
 		config.ChainID(),
-		addr,
 		types.NewMockPV(),
-		ed25519.GenPrivKey(),
+		dialer,
 	)
-	privval.RemoteSignerConnDeadline(5 * time.Millisecond)(rs)
+
 	go func() {
-		err := rs.Start()
+		err := pvsc.Start()
 		if err != nil {
 			panic(err)
 		}
 	}()
-	defer rs.Stop()
+	defer pvsc.Stop()
 
 	n, err := DefaultNewNode(config, log.TestingLogger())
 	require.NoError(t, err)
-	assert.IsType(t, &privval.TCPVal{}, n.PrivValidator())
+	assert.IsType(t, &privval.SocketVal{}, n.PrivValidator())
 }
 
 // address without a protocol must result in error
@@ -161,25 +161,25 @@ func TestNodeSetPrivValIPC(t *testing.T) {
 	config := cfg.ResetTestRoot("node_priv_val_tcp_test")
 	config.BaseConfig.PrivValidatorListenAddr = "unix://" + tmpfile
 
-	rs := privval.NewIPCRemoteSigner(
+	dialer := privval.DialUnixFn(tmpfile)
+	pvsc := privval.NewRemoteSigner(
 		log.TestingLogger(),
 		config.ChainID(),
-		tmpfile,
 		types.NewMockPV(),
+		dialer,
 	)
-	privval.IPCRemoteSignerConnDeadline(3 * time.Second)(rs)
 
 	done := make(chan struct{})
 	go func() {
 		defer close(done)
 		n, err := DefaultNewNode(config, log.TestingLogger())
 		require.NoError(t, err)
-		assert.IsType(t, &privval.IPCVal{}, n.PrivValidator())
+		assert.IsType(t, &privval.SocketVal{}, n.PrivValidator())
 	}()
 
-	err := rs.Start()
+	err := pvsc.Start()
 	require.NoError(t, err)
-	defer rs.Stop()
+	defer pvsc.Stop()
 
 	<-done
 }

p2p/README.md

@@ -4,8 +4,8 @@ The p2p package provides an abstraction around peer-to-peer communication.
 
 Docs:
 
-- [Connection](https://github.com/tendermint/tendermint/blob/master/docs/spec/docs/spec/p2p/connection.md) for details on how connections and multiplexing work
-- [Peer](https://github.com/tendermint/tendermint/blob/master/docs/spec/docs/spec/p2p/peer.md) for details on peer ID, handshakes, and peer exchange
-- [Node](https://github.com/tendermint/tendermint/blob/master/docs/spec/docs/spec/p2p/node.md) for details about different types of nodes and how they should work
-- [Pex](https://github.com/tendermint/tendermint/blob/master/docs/spec/docs/spec/reactors/pex/pex.md) for details on peer discovery and exchange
-- [Config](https://github.com/tendermint/tendermint/blob/master/docs/spec/docs/spec/p2p/config.md) for details on some config option
+- [Connection](https://github.com/tendermint/tendermint/blob/master/docs/spec/p2p/connection.md) for details on how connections and multiplexing work
+- [Peer](https://github.com/tendermint/tendermint/blob/master/docs/spec/p2p/peer.md) for details on peer ID, handshakes, and peer exchange
+- [Node](https://github.com/tendermint/tendermint/blob/master/docs/spec/p2p/node.md) for details about different types of nodes and how they should work
+- [Pex](https://github.com/tendermint/tendermint/blob/master/docs/spec/reactors/pex/pex.md) for details on peer discovery and exchange
+- [Config](https://github.com/tendermint/tendermint/blob/master/docs/spec/p2p/config.md) for details on some config option

p2p/conn/secret_connection.go

@@ -8,6 +8,7 @@ import (
 	"errors"
 	"io"
 	"net"
+	"sync"
 	"time"
 
 	"golang.org/x/crypto/chacha20poly1305"
@@ -27,20 +28,36 @@ const aeadSizeOverhead = 16 // overhead of poly 1305 authentication tag
 const aeadKeySize = chacha20poly1305.KeySize
 const aeadNonceSize = chacha20poly1305.NonceSize
 
-// SecretConnection implements net.conn.
+// SecretConnection implements net.Conn.
 // It is an implementation of the STS protocol.
-// Note we do not (yet) assume that a remote peer's pubkey
-// is known ahead of time, and thus we are technically
-// still vulnerable to MITM. (TODO!)
-// See docs/sts-final.pdf for more info
+// See https://github.com/tendermint/tendermint/blob/0.1/docs/sts-final.pdf for
+// details on the protocol.
+//
+// Consumers of the SecretConnection are responsible for authenticating
+// the remote peer's pubkey against known information, like a nodeID.
+// Otherwise they are vulnerable to MITM.
+// (TODO(ismail): see also https://github.com/tendermint/tendermint/issues/3010)
 type SecretConnection struct {
-	conn       io.ReadWriteCloser
-	recvBuffer []byte
-	recvNonce  *[aeadNonceSize]byte
-	sendNonce  *[aeadNonceSize]byte
+
+	// immutable
 	recvSecret *[aeadKeySize]byte
 	sendSecret *[aeadKeySize]byte
 	remPubKey  crypto.PubKey
+	conn       io.ReadWriteCloser
+
+	// net.Conn must be thread safe:
+	// https://golang.org/pkg/net/#Conn.
+	// Since we have internal mutable state,
+	// we need mtxs. But recv and send states
+	// are independent, so we can use two mtxs.
+	// All .Read are covered by recvMtx,
+	// all .Write are covered by sendMtx.
+	recvMtx    sync.Mutex
+	recvBuffer []byte
+	recvNonce  *[aeadNonceSize]byte
+
+	sendMtx   sync.Mutex
+	sendNonce *[aeadNonceSize]byte
 }
 
 // MakeSecretConnection performs handshake and returns a new authenticated
@@ -109,9 +126,12 @@ func (sc *SecretConnection) RemotePubKey() crypto.PubKey {
 	return sc.remPubKey
 }
 
-// Writes encrypted frames of `sealedFrameSize`
-// CONTRACT: data smaller than dataMaxSize is read atomically.
+// Writes encrypted frames of `totalFrameSize + aeadSizeOverhead`.
+// CONTRACT: data smaller than dataMaxSize is written atomically.
 func (sc *SecretConnection) Write(data []byte) (n int, err error) {
+	sc.sendMtx.Lock()
+	defer sc.sendMtx.Unlock()
+
 	for 0 < len(data) {
 		var frame = make([]byte, totalFrameSize)
 		var chunk []byte
@@ -130,6 +150,7 @@ func (sc *SecretConnection) Write(data []byte) (n int, err error) {
 		if err != nil {
 			return n, errors.New("Invalid SecretConnection Key")
 		}
+
 		// encrypt the frame
 		var sealedFrame = make([]byte, aeadSizeOverhead+totalFrameSize)
 		aead.Seal(sealedFrame[:0], sc.sendNonce[:], frame, nil)
@@ -147,23 +168,30 @@ func (sc *SecretConnection) Write(data []byte) (n int, err error) {
 
 // CONTRACT: data smaller than dataMaxSize is read atomically.
 func (sc *SecretConnection) Read(data []byte) (n int, err error) {
+	sc.recvMtx.Lock()
+	defer sc.recvMtx.Unlock()
+
+	// read off and update the recvBuffer, if non-empty
 	if 0 < len(sc.recvBuffer) {
 		n = copy(data, sc.recvBuffer)
 		sc.recvBuffer = sc.recvBuffer[n:]
 		return
 	}
 
+	// read off the conn
+	sealedFrame := make([]byte, totalFrameSize+aeadSizeOverhead)
+	_, err = io.ReadFull(sc.conn, sealedFrame)
+	if err != nil {
+		return
+	}
+
 	aead, err := chacha20poly1305.New(sc.recvSecret[:])
 	if err != nil {
 		return n, errors.New("Invalid SecretConnection Key")
 	}
-	sealedFrame := make([]byte, totalFrameSize+aeadSizeOverhead)
-	_, err = io.ReadFull(sc.conn, sealedFrame)
-	if err != nil {
-		return
-	}
 
-	// decrypt the frame
+	// decrypt the frame.
+	// reads and updates the sc.recvNonce
 	var frame = make([]byte, totalFrameSize)
 	_, err = aead.Open(frame[:0], sc.recvNonce[:], sealedFrame, nil)
 	if err != nil {
@@ -172,12 +200,13 @@ func (sc *SecretConnection) Read(data []byte) (n int, err error) {
 	incrNonce(sc.recvNonce)
 	// end decryption
 
+	// copy checkLength worth into data,
+	// set recvBuffer to the rest.
 	var chunkLength = binary.LittleEndian.Uint32(frame) // read the first four bytes
 	if chunkLength > dataMaxSize {
 		return 0, errors.New("chunkLength is greater than dataMaxSize")
 	}
 	var chunk = frame[dataLenSize : dataLenSize+chunkLength]
-
 	n = copy(data, chunk)
 	sc.recvBuffer = chunk[n:]
 	return

p2p/conn/secret_connection_test.go

@@ -7,10 +7,12 @@ import (
 	"fmt"
 	"io"
 	"log"
+	"net"
 	"os"
 	"path/filepath"
 	"strconv"
 	"strings"
+	"sync"
 	"testing"
 
 	"github.com/stretchr/testify/assert"
@@ -98,6 +100,69 @@ func TestSecretConnectionHandshake(t *testing.T) {
 	}
 }
 
+func TestConcurrentWrite(t *testing.T) {
+	fooSecConn, barSecConn := makeSecretConnPair(t)
+	fooWriteText := cmn.RandStr(dataMaxSize)
+
+	// write from two routines.
+	// should be safe from race according to net.Conn:
+	// https://golang.org/pkg/net/#Conn
+	n := 100
+	wg := new(sync.WaitGroup)
+	wg.Add(3)
+	go writeLots(t, wg, fooSecConn, fooWriteText, n)
+	go writeLots(t, wg, fooSecConn, fooWriteText, n)
+
+	// Consume reads from bar's reader
+	readLots(t, wg, barSecConn, n*2)
+	wg.Wait()
+
+	if err := fooSecConn.Close(); err != nil {
+		t.Error(err)
+	}
+}
+
+func TestConcurrentRead(t *testing.T) {
+	fooSecConn, barSecConn := makeSecretConnPair(t)
+	fooWriteText := cmn.RandStr(dataMaxSize)
+	n := 100
+
+	// read from two routines.
+	// should be safe from race according to net.Conn:
+	// https://golang.org/pkg/net/#Conn
+	wg := new(sync.WaitGroup)
+	wg.Add(3)
+	go readLots(t, wg, fooSecConn, n/2)
+	go readLots(t, wg, fooSecConn, n/2)
+
+	// write to bar
+	writeLots(t, wg, barSecConn, fooWriteText, n)
+	wg.Wait()
+
+	if err := fooSecConn.Close(); err != nil {
+		t.Error(err)
+	}
+}
+
+func writeLots(t *testing.T, wg *sync.WaitGroup, conn net.Conn, txt string, n int) {
+	defer wg.Done()
+	for i := 0; i < n; i++ {
+		_, err := conn.Write([]byte(txt))
+		if err != nil {
+			t.Fatalf("Failed to write to fooSecConn: %v", err)
+		}
+	}
+}
+
+func readLots(t *testing.T, wg *sync.WaitGroup, conn net.Conn, n int) {
+	readBuffer := make([]byte, dataMaxSize)
+	for i := 0; i < n; i++ {
+		_, err := conn.Read(readBuffer)
+		assert.NoError(t, err)
+	}
+	wg.Done()
+}
+
 func TestSecretConnectionReadWrite(t *testing.T) {
 	fooConn, barConn := makeKVStoreConnPair()
 	fooWrites, barWrites := []string{}, []string{}

p2p/metrics.go

@@ -7,7 +7,11 @@ import (
 	stdprometheus "github.com/prometheus/client_golang/prometheus"
 )
 
-const MetricsSubsystem = "p2p"
+const (
+	// MetricsSubsystem is a subsystem shared by all metrics exposed by this
+	// package.
+	MetricsSubsystem = "p2p"
+)
 
 // Metrics contains metrics exposed by this package.
 type Metrics struct {
@@ -24,38 +28,44 @@ type Metrics struct {
 }
 
 // PrometheusMetrics returns Metrics build using Prometheus client library.
-func PrometheusMetrics(namespace string) *Metrics {
+// Optionally, labels can be provided along with their values ("foo",
+// "fooValue").
+func PrometheusMetrics(namespace string, labelsAndValues ...string) *Metrics {
+	labels := []string{}
+	for i := 0; i < len(labelsAndValues); i += 2 {
+		labels = append(labels, labelsAndValues[i])
+	}
 	return &Metrics{
 		Peers: prometheus.NewGaugeFrom(stdprometheus.GaugeOpts{
 			Namespace: namespace,
 			Subsystem: MetricsSubsystem,
 			Name:      "peers",
 			Help:      "Number of peers.",
-		}, []string{}),
+		}, labels).With(labelsAndValues...),
 		PeerReceiveBytesTotal: prometheus.NewCounterFrom(stdprometheus.CounterOpts{
 			Namespace: namespace,
 			Subsystem: MetricsSubsystem,
 			Name:      "peer_receive_bytes_total",
 			Help:      "Number of bytes received from a given peer.",
-		}, []string{"peer_id"}),
+		}, append(labels, "peer_id")).With(labelsAndValues...),
 		PeerSendBytesTotal: prometheus.NewCounterFrom(stdprometheus.CounterOpts{
 			Namespace: namespace,
 			Subsystem: MetricsSubsystem,
 			Name:      "peer_send_bytes_total",
 			Help:      "Number of bytes sent to a given peer.",
-		}, []string{"peer_id"}),
+		}, append(labels, "peer_id")).With(labelsAndValues...),
 		PeerPendingSendBytes: prometheus.NewGaugeFrom(stdprometheus.GaugeOpts{
 			Namespace: namespace,
 			Subsystem: MetricsSubsystem,
 			Name:      "peer_pending_send_bytes",
 			Help:      "Number of pending bytes to be sent to a given peer.",
-		}, []string{"peer_id"}),
+		}, append(labels, "peer_id")).With(labelsAndValues...),
 		NumTxs: prometheus.NewGaugeFrom(stdprometheus.GaugeOpts{
 			Namespace: namespace,
 			Subsystem: MetricsSubsystem,
 			Name:      "num_txs",
 			Help:      "Number of transactions submitted by each peer.",
-		}, []string{"peer_id"}),
+		}, append(labels, "peer_id")).With(labelsAndValues...),
 	}
 }
 

privval/client.go

@@ -0,0 +1,238 @@
+package privval
+
+import (
+	"errors"
+	"fmt"
+	"net"
+	"sync"
+	"time"
+
+	"github.com/tendermint/tendermint/crypto"
+	cmn "github.com/tendermint/tendermint/libs/common"
+	"github.com/tendermint/tendermint/libs/log"
+	"github.com/tendermint/tendermint/types"
+)
+
+const (
+	defaultConnHeartBeatSeconds = 2
+	defaultDialRetries          = 10
+)
+
+// Socket errors.
+var (
+	ErrUnexpectedResponse = errors.New("received unexpected response")
+)
+
+var (
+	connHeartbeat = time.Second * defaultConnHeartBeatSeconds
+)
+
+// SocketValOption sets an optional parameter on the SocketVal.
+type SocketValOption func(*SocketVal)
+
+// SocketValHeartbeat sets the period on which to check the liveness of the
+// connected Signer connections.
+func SocketValHeartbeat(period time.Duration) SocketValOption {
+	return func(sc *SocketVal) { sc.connHeartbeat = period }
+}
+
+// SocketVal implements PrivValidator.
+// It listens for an external process to dial in and uses
+// the socket to request signatures.
+type SocketVal struct {
+	cmn.BaseService
+
+	listener net.Listener
+
+	// ping
+	cancelPing    chan struct{}
+	pingTicker    *time.Ticker
+	connHeartbeat time.Duration
+
+	// signer is mutable since it can be
+	// reset if the connection fails.
+	// failures are detected by a background
+	// ping routine.
+	// Methods on the underlying net.Conn itself
+	// are already gorountine safe.
+	mtx    sync.RWMutex
+	signer *RemoteSignerClient
+}
+
+// Check that SocketVal implements PrivValidator.
+var _ types.PrivValidator = (*SocketVal)(nil)
+
+// NewSocketVal returns an instance of SocketVal.
+func NewSocketVal(
+	logger log.Logger,
+	listener net.Listener,
+) *SocketVal {
+	sc := &SocketVal{
+		listener:      listener,
+		connHeartbeat: connHeartbeat,
+	}
+
+	sc.BaseService = *cmn.NewBaseService(logger, "SocketVal", sc)
+
+	return sc
+}
+
+//--------------------------------------------------------
+// Implement PrivValidator
+
+// GetPubKey implements PrivValidator.
+func (sc *SocketVal) GetPubKey() crypto.PubKey {
+	sc.mtx.RLock()
+	defer sc.mtx.RUnlock()
+	return sc.signer.GetPubKey()
+}
+
+// SignVote implements PrivValidator.
+func (sc *SocketVal) SignVote(chainID string, vote *types.Vote) error {
+	sc.mtx.RLock()
+	defer sc.mtx.RUnlock()
+	return sc.signer.SignVote(chainID, vote)
+}
+
+// SignProposal implements PrivValidator.
+func (sc *SocketVal) SignProposal(chainID string, proposal *types.Proposal) error {
+	sc.mtx.RLock()
+	defer sc.mtx.RUnlock()
+	return sc.signer.SignProposal(chainID, proposal)
+}
+
+//--------------------------------------------------------
+// More thread safe methods proxied to the signer
+
+// Ping is used to check connection health.
+func (sc *SocketVal) Ping() error {
+	sc.mtx.RLock()
+	defer sc.mtx.RUnlock()
+	return sc.signer.Ping()
+}
+
+// Close closes the underlying net.Conn.
+func (sc *SocketVal) Close() {
+	sc.mtx.RLock()
+	defer sc.mtx.RUnlock()
+	if sc.signer != nil {
+		if err := sc.signer.Close(); err != nil {
+			sc.Logger.Error("OnStop", "err", err)
+		}
+	}
+
+	if sc.listener != nil {
+		if err := sc.listener.Close(); err != nil {
+			sc.Logger.Error("OnStop", "err", err)
+		}
+	}
+}
+
+//--------------------------------------------------------
+// Service start and stop
+
+// OnStart implements cmn.Service.
+func (sc *SocketVal) OnStart() error {
+	if closed, err := sc.reset(); err != nil {
+		sc.Logger.Error("OnStart", "err", err)
+		return err
+	} else if closed {
+		return fmt.Errorf("listener is closed")
+	}
+
+	// Start a routine to keep the connection alive
+	sc.cancelPing = make(chan struct{}, 1)
+	sc.pingTicker = time.NewTicker(sc.connHeartbeat)
+	go func() {
+		for {
+			select {
+			case <-sc.pingTicker.C:
+				err := sc.Ping()
+				if err != nil {
+					sc.Logger.Error("Ping", "err", err)
+					if err == ErrUnexpectedResponse {
+						return
+					}
+
+					closed, err := sc.reset()
+					if err != nil {
+						sc.Logger.Error("Reconnecting to remote signer failed", "err", err)
+						continue
+					}
+					if closed {
+						sc.Logger.Info("listener is closing")
+						return
+					}
+
+					sc.Logger.Info("Re-created connection to remote signer", "impl", sc)
+				}
+			case <-sc.cancelPing:
+				sc.pingTicker.Stop()
+				return
+			}
+		}
+	}()
+
+	return nil
+}
+
+// OnStop implements cmn.Service.
+func (sc *SocketVal) OnStop() {
+	if sc.cancelPing != nil {
+		close(sc.cancelPing)
+	}
+	sc.Close()
+}
+
+//--------------------------------------------------------
+// Connection and signer management
+
+// waits to accept and sets a new connection.
+// connection is closed in OnStop.
+// returns true if the listener is closed
+// (ie. it returns a nil conn).
+func (sc *SocketVal) reset() (closed bool, err error) {
+	sc.mtx.Lock()
+	defer sc.mtx.Unlock()
+
+	// first check if the conn already exists and close it.
+	if sc.signer != nil {
+		if err := sc.signer.Close(); err != nil {
+			sc.Logger.Error("error closing socket val connection during reset", "err", err)
+		}
+	}
+
+	// wait for a new conn
+	conn, err := sc.acceptConnection()
+	if err != nil {
+		return false, err
+	}
+
+	// listener is closed
+	if conn == nil {
+		return true, nil
+	}
+
+	sc.signer, err = NewRemoteSignerClient(conn)
+	if err != nil {
+		// failed to fetch the pubkey. close out the connection.
+		if err := conn.Close(); err != nil {
+			sc.Logger.Error("error closing connection", "err", err)
+		}
+		return false, err
+	}
+	return false, nil
+}
+
+// Attempt to accept a connection.
+// Times out after the listener's acceptDeadline
+func (sc *SocketVal) acceptConnection() (net.Conn, error) {
+	conn, err := sc.listener.Accept()
+	if err != nil {
+		if !sc.IsRunning() {
+			return nil, nil // Ignore error from listener closing.
+		}
+		return nil, err
+	}
+	return conn, nil
+}

privval/client_test.go

@@ -0,0 +1,461 @@
+package privval
+
+import (
+	"fmt"
+	"net"
+	"testing"
+	"time"
+
+	"github.com/stretchr/testify/assert"
+	"github.com/stretchr/testify/require"
+
+	"github.com/tendermint/tendermint/crypto/ed25519"
+	cmn "github.com/tendermint/tendermint/libs/common"
+	"github.com/tendermint/tendermint/libs/log"
+
+	"github.com/tendermint/tendermint/types"
+)
+
+var (
+	testAcceptDeadline = defaultAcceptDeadlineSeconds * time.Second
+
+	testConnDeadline    = 100 * time.Millisecond
+	testConnDeadline2o3 = 66 * time.Millisecond // 2/3 of the other one
+
+	testHeartbeatTimeout    = 10 * time.Millisecond
+	testHeartbeatTimeout3o2 = 6 * time.Millisecond // 3/2 of the other one
+)
+
+type socketTestCase struct {
+	addr   string
+	dialer Dialer
+}
+
+func socketTestCases(t *testing.T) []socketTestCase {
+	tcpAddr := fmt.Sprintf("tcp://%s", testFreeTCPAddr(t))
+	unixFilePath, err := testUnixAddr()
+	require.NoError(t, err)
+	unixAddr := fmt.Sprintf("unix://%s", unixFilePath)
+	return []socketTestCase{
+		socketTestCase{
+			addr:   tcpAddr,
+			dialer: DialTCPFn(tcpAddr, testConnDeadline, ed25519.GenPrivKey()),
+		},
+		socketTestCase{
+			addr:   unixAddr,
+			dialer: DialUnixFn(unixFilePath),
+		},
+	}
+}
+
+func TestSocketPVAddress(t *testing.T) {
+	for _, tc := range socketTestCases(t) {
+		// Execute the test within a closure to ensure the deferred statements
+		// are called between each for loop iteration, for isolated test cases.
+		func() {
+			var (
+				chainID = cmn.RandStr(12)
+				sc, rs  = testSetupSocketPair(t, chainID, types.NewMockPV(), tc.addr, tc.dialer)
+			)
+			defer sc.Stop()
+			defer rs.Stop()
+
+			serverAddr := rs.privVal.GetPubKey().Address()
+			clientAddr := sc.GetPubKey().Address()
+
+			assert.Equal(t, serverAddr, clientAddr)
+		}()
+	}
+}
+
+func TestSocketPVPubKey(t *testing.T) {
+	for _, tc := range socketTestCases(t) {
+		func() {
+			var (
+				chainID = cmn.RandStr(12)
+				sc, rs  = testSetupSocketPair(t, chainID, types.NewMockPV(), tc.addr, tc.dialer)
+			)
+			defer sc.Stop()
+			defer rs.Stop()
+
+			clientKey := sc.GetPubKey()
+
+			privvalPubKey := rs.privVal.GetPubKey()
+
+			assert.Equal(t, privvalPubKey, clientKey)
+		}()
+	}
+}
+
+func TestSocketPVProposal(t *testing.T) {
+	for _, tc := range socketTestCases(t) {
+		func() {
+			var (
+				chainID = cmn.RandStr(12)
+				sc, rs  = testSetupSocketPair(t, chainID, types.NewMockPV(), tc.addr, tc.dialer)
+
+				ts             = time.Now()
+				privProposal   = &types.Proposal{Timestamp: ts}
+				clientProposal = &types.Proposal{Timestamp: ts}
+			)
+			defer sc.Stop()
+			defer rs.Stop()
+
+			require.NoError(t, rs.privVal.SignProposal(chainID, privProposal))
+			require.NoError(t, sc.SignProposal(chainID, clientProposal))
+			assert.Equal(t, privProposal.Signature, clientProposal.Signature)
+		}()
+	}
+}
+
+func TestSocketPVVote(t *testing.T) {
+	for _, tc := range socketTestCases(t) {
+		func() {
+			var (
+				chainID = cmn.RandStr(12)
+				sc, rs  = testSetupSocketPair(t, chainID, types.NewMockPV(), tc.addr, tc.dialer)
+
+				ts    = time.Now()
+				vType = types.PrecommitType
+				want  = &types.Vote{Timestamp: ts, Type: vType}
+				have  = &types.Vote{Timestamp: ts, Type: vType}
+			)
+			defer sc.Stop()
+			defer rs.Stop()
+
+			require.NoError(t, rs.privVal.SignVote(chainID, want))
+			require.NoError(t, sc.SignVote(chainID, have))
+			assert.Equal(t, want.Signature, have.Signature)
+		}()
+	}
+}
+
+func TestSocketPVVoteResetDeadline(t *testing.T) {
+	for _, tc := range socketTestCases(t) {
+		func() {
+			var (
+				chainID = cmn.RandStr(12)
+				sc, rs  = testSetupSocketPair(t, chainID, types.NewMockPV(), tc.addr, tc.dialer)
+
+				ts    = time.Now()
+				vType = types.PrecommitType
+				want  = &types.Vote{Timestamp: ts, Type: vType}
+				have  = &types.Vote{Timestamp: ts, Type: vType}
+			)
+			defer sc.Stop()
+			defer rs.Stop()
+
+			time.Sleep(testConnDeadline2o3)
+
+			require.NoError(t, rs.privVal.SignVote(chainID, want))
+			require.NoError(t, sc.SignVote(chainID, have))
+			assert.Equal(t, want.Signature, have.Signature)
+
+			// This would exceed the deadline if it was not extended by the previous message
+			time.Sleep(testConnDeadline2o3)
+
+			require.NoError(t, rs.privVal.SignVote(chainID, want))
+			require.NoError(t, sc.SignVote(chainID, have))
+			assert.Equal(t, want.Signature, have.Signature)
+		}()
+	}
+}
+
+func TestSocketPVVoteKeepalive(t *testing.T) {
+	for _, tc := range socketTestCases(t) {
+		func() {
+			var (
+				chainID = cmn.RandStr(12)
+				sc, rs  = testSetupSocketPair(t, chainID, types.NewMockPV(), tc.addr, tc.dialer)
+
+				ts    = time.Now()
+				vType = types.PrecommitType
+				want  = &types.Vote{Timestamp: ts, Type: vType}
+				have  = &types.Vote{Timestamp: ts, Type: vType}
+			)
+			defer sc.Stop()
+			defer rs.Stop()
+
+			time.Sleep(testConnDeadline * 2)
+
+			require.NoError(t, rs.privVal.SignVote(chainID, want))
+			require.NoError(t, sc.SignVote(chainID, have))
+			assert.Equal(t, want.Signature, have.Signature)
+		}()
+	}
+}
+
+func TestSocketPVDeadline(t *testing.T) {
+	for _, tc := range socketTestCases(t) {
+		func() {
+			var (
+				listenc         = make(chan struct{})
+				thisConnTimeout = 100 * time.Millisecond
+				sc              = newSocketVal(log.TestingLogger(), tc.addr, thisConnTimeout)
+			)
+
+			go func(sc *SocketVal) {
+				defer close(listenc)
+
+				// Note: the TCP connection times out at the accept() phase,
+				// whereas the Unix domain sockets connection times out while
+				// attempting to fetch the remote signer's public key.
+				assert.True(t, IsConnTimeout(sc.Start()))
+
+				assert.False(t, sc.IsRunning())
+			}(sc)
+
+			for {
+				_, err := cmn.Connect(tc.addr)
+				if err == nil {
+					break
+				}
+			}
+
+			<-listenc
+		}()
+	}
+}
+
+func TestRemoteSignVoteErrors(t *testing.T) {
+	for _, tc := range socketTestCases(t) {
+		func() {
+			var (
+				chainID = cmn.RandStr(12)
+				sc, rs  = testSetupSocketPair(t, chainID, types.NewErroringMockPV(), tc.addr, tc.dialer)
+
+				ts    = time.Now()
+				vType = types.PrecommitType
+				vote  = &types.Vote{Timestamp: ts, Type: vType}
+			)
+			defer sc.Stop()
+			defer rs.Stop()
+
+			err := sc.SignVote("", vote)
+			require.Equal(t, err.(*RemoteSignerError).Description, types.ErroringMockPVErr.Error())
+
+			err = rs.privVal.SignVote(chainID, vote)
+			require.Error(t, err)
+			err = sc.SignVote(chainID, vote)
+			require.Error(t, err)
+		}()
+	}
+}
+
+func TestRemoteSignProposalErrors(t *testing.T) {
+	for _, tc := range socketTestCases(t) {
+		func() {
+			var (
+				chainID = cmn.RandStr(12)
+				sc, rs  = testSetupSocketPair(t, chainID, types.NewErroringMockPV(), tc.addr, tc.dialer)
+
+				ts       = time.Now()
+				proposal = &types.Proposal{Timestamp: ts}
+			)
+			defer sc.Stop()
+			defer rs.Stop()
+
+			err := sc.SignProposal("", proposal)
+			require.Equal(t, err.(*RemoteSignerError).Description, types.ErroringMockPVErr.Error())
+
+			err = rs.privVal.SignProposal(chainID, proposal)
+			require.Error(t, err)
+
+			err = sc.SignProposal(chainID, proposal)
+			require.Error(t, err)
+		}()
+	}
+}
+
+func TestErrUnexpectedResponse(t *testing.T) {
+	for _, tc := range socketTestCases(t) {
+		func() {
+			var (
+				logger  = log.TestingLogger()
+				chainID = cmn.RandStr(12)
+				readyc  = make(chan struct{})
+				errc    = make(chan error, 1)
+
+				rs = NewRemoteSigner(
+					logger,
+					chainID,
+					types.NewMockPV(),
+					tc.dialer,
+				)
+				sc = newSocketVal(logger, tc.addr, testConnDeadline)
+			)
+
+			testStartSocketPV(t, readyc, sc)
+			defer sc.Stop()
+			RemoteSignerConnDeadline(time.Millisecond)(rs)
+			RemoteSignerConnRetries(100)(rs)
+			// we do not want to Start() the remote signer here and instead use the connection to
+			// reply with intentionally wrong replies below:
+			rsConn, err := rs.connect()
+			defer rsConn.Close()
+			require.NoError(t, err)
+			require.NotNil(t, rsConn)
+			// send over public key to get the remote signer running:
+			go testReadWriteResponse(t, &PubKeyResponse{}, rsConn)
+			<-readyc
+
+			// Proposal:
+			go func(errc chan error) {
+				errc <- sc.SignProposal(chainID, &types.Proposal{})
+			}(errc)
+			// read request and write wrong response:
+			go testReadWriteResponse(t, &SignedVoteResponse{}, rsConn)
+			err = <-errc
+			require.Error(t, err)
+			require.Equal(t, err, ErrUnexpectedResponse)
+
+			// Vote:
+			go func(errc chan error) {
+				errc <- sc.SignVote(chainID, &types.Vote{})
+			}(errc)
+			// read request and write wrong response:
+			go testReadWriteResponse(t, &SignedProposalResponse{}, rsConn)
+			err = <-errc
+			require.Error(t, err)
+			require.Equal(t, err, ErrUnexpectedResponse)
+		}()
+	}
+}
+
+func TestRetryConnToRemoteSigner(t *testing.T) {
+	for _, tc := range socketTestCases(t) {
+		func() {
+			var (
+				logger  = log.TestingLogger()
+				chainID = cmn.RandStr(12)
+				readyc  = make(chan struct{})
+
+				rs = NewRemoteSigner(
+					logger,
+					chainID,
+					types.NewMockPV(),
+					tc.dialer,
+				)
+				thisConnTimeout = testConnDeadline
+				sc              = newSocketVal(logger, tc.addr, thisConnTimeout)
+			)
+			// Ping every:
+			SocketValHeartbeat(testHeartbeatTimeout)(sc)
+
+			RemoteSignerConnDeadline(testConnDeadline)(rs)
+			RemoteSignerConnRetries(10)(rs)
+
+			testStartSocketPV(t, readyc, sc)
+			defer sc.Stop()
+			require.NoError(t, rs.Start())
+			assert.True(t, rs.IsRunning())
+
+			<-readyc
+			time.Sleep(testHeartbeatTimeout * 2)
+
+			rs.Stop()
+			rs2 := NewRemoteSigner(
+				logger,
+				chainID,
+				types.NewMockPV(),
+				tc.dialer,
+			)
+			// let some pings pass
+			time.Sleep(testHeartbeatTimeout3o2)
+			require.NoError(t, rs2.Start())
+			assert.True(t, rs2.IsRunning())
+			defer rs2.Stop()
+
+			// give the client some time to re-establish the conn to the remote signer
+			// should see sth like this in the logs:
+			//
+			// E[10016-01-10|17:12:46.128] Ping                                         err="remote signer timed out"
+			// I[10016-01-10|17:16:42.447] Re-created connection to remote signer       impl=SocketVal
+			time.Sleep(testConnDeadline * 2)
+		}()
+	}
+}
+
+func newSocketVal(logger log.Logger, addr string, connDeadline time.Duration) *SocketVal {
+	proto, address := cmn.ProtocolAndAddress(addr)
+	ln, err := net.Listen(proto, address)
+	logger.Info("Listening at", "proto", proto, "address", address)
+	if err != nil {
+		panic(err)
+	}
+	var svln net.Listener
+	if proto == "unix" {
+		unixLn := NewUnixListener(ln)
+		UnixListenerAcceptDeadline(testAcceptDeadline)(unixLn)
+		UnixListenerConnDeadline(connDeadline)(unixLn)
+		svln = unixLn
+	} else {
+		tcpLn := NewTCPListener(ln, ed25519.GenPrivKey())
+		TCPListenerAcceptDeadline(testAcceptDeadline)(tcpLn)
+		TCPListenerConnDeadline(connDeadline)(tcpLn)
+		svln = tcpLn
+	}
+	return NewSocketVal(logger, svln)
+}
+
+func testSetupSocketPair(
+	t *testing.T,
+	chainID string,
+	privValidator types.PrivValidator,
+	addr string,
+	dialer Dialer,
+) (*SocketVal, *RemoteSigner) {
+	var (
+		logger  = log.TestingLogger()
+		privVal = privValidator
+		readyc  = make(chan struct{})
+		rs      = NewRemoteSigner(
+			logger,
+			chainID,
+			privVal,
+			dialer,
+		)
+
+		thisConnTimeout = testConnDeadline
+		sc              = newSocketVal(logger, addr, thisConnTimeout)
+	)
+
+	SocketValHeartbeat(testHeartbeatTimeout)(sc)
+	RemoteSignerConnDeadline(testConnDeadline)(rs)
+	RemoteSignerConnRetries(1e6)(rs)
+
+	testStartSocketPV(t, readyc, sc)
+
+	require.NoError(t, rs.Start())
+	assert.True(t, rs.IsRunning())
+
+	<-readyc
+
+	return sc, rs
+}
+
+func testReadWriteResponse(t *testing.T, resp RemoteSignerMsg, rsConn net.Conn) {
+	_, err := readMsg(rsConn)
+	require.NoError(t, err)
+
+	err = writeMsg(rsConn, resp)
+	require.NoError(t, err)
+}
+
+func testStartSocketPV(t *testing.T, readyc chan struct{}, sc *SocketVal) {
+	go func(sc *SocketVal) {
+		require.NoError(t, sc.Start())
+		assert.True(t, sc.IsRunning())
+
+		readyc <- struct{}{}
+	}(sc)
+}
+
+// testFreeTCPAddr claims a free port so we don't block on listener being ready.
+func testFreeTCPAddr(t *testing.T) string {
+	ln, err := net.Listen("tcp", "127.0.0.1:0")
+	require.NoError(t, err)
+	defer ln.Close()
+
+	return fmt.Sprintf("127.0.0.1:%d", ln.Addr().(*net.TCPAddr).Port)
+}

privval/doc.go

@@ -0,0 +1,21 @@
+/*
+
+Package privval provides different implementations of the types.PrivValidator.
+
+FilePV
+
+FilePV is the simplest implementation and developer default. It uses one file for the private key and another to store state.
+
+SocketVal
+
+SocketVal establishes a connection to an external process, like a Key Management Server (KMS), using a socket.
+SocketVal listens for the external KMS process to dial in.
+SocketVal takes a listener, which determines the type of connection
+(ie. encrypted over tcp, or unencrypted over unix).
+
+RemoteSigner
+
+RemoteSigner is a simple wrapper around a net.Conn. It's used by both IPCVal and TCPVal.
+
+*/
+package privval

privval/file.go

@@ -22,6 +22,7 @@ const (
 	stepPrecommit int8 = 3
 )
 
+// A vote is either stepPrevote or stepPrecommit.
 func voteToStep(vote *types.Vote) int8 {
 	switch vote.Type {
 	case types.PrevoteType:
@@ -29,7 +30,7 @@ func voteToStep(vote *types.Vote) int8 {
 	case types.PrecommitType:
 		return stepPrecommit
 	default:
-		cmn.PanicSanity("Unknown vote type")
+		panic("Unknown vote type")
 		return 0
 	}
 }

privval/ipc.go

@@ -1,123 +0,0 @@
-package privval
-
-import (
-	"net"
-	"time"
-
-	cmn "github.com/tendermint/tendermint/libs/common"
-	"github.com/tendermint/tendermint/libs/log"
-	"github.com/tendermint/tendermint/types"
-)
-
-// IPCValOption sets an optional parameter on the SocketPV.
-type IPCValOption func(*IPCVal)
-
-// IPCValConnTimeout sets the read and write timeout for connections
-// from external signing processes.
-func IPCValConnTimeout(timeout time.Duration) IPCValOption {
-	return func(sc *IPCVal) { sc.connTimeout = timeout }
-}
-
-// IPCValHeartbeat sets the period on which to check the liveness of the
-// connected Signer connections.
-func IPCValHeartbeat(period time.Duration) IPCValOption {
-	return func(sc *IPCVal) { sc.connHeartbeat = period }
-}
-
-// IPCVal implements PrivValidator, it uses a unix socket to request signatures
-// from an external process.
-type IPCVal struct {
-	cmn.BaseService
-	*RemoteSignerClient
-
-	addr string
-
-	connTimeout   time.Duration
-	connHeartbeat time.Duration
-
-	conn       net.Conn
-	cancelPing chan struct{}
-	pingTicker *time.Ticker
-}
-
-// Check that IPCVal implements PrivValidator.
-var _ types.PrivValidator = (*IPCVal)(nil)
-
-// NewIPCVal returns an instance of IPCVal.
-func NewIPCVal(
-	logger log.Logger,
-	socketAddr string,
-) *IPCVal {
-	sc := &IPCVal{
-		addr:          socketAddr,
-		connTimeout:   connTimeout,
-		connHeartbeat: connHeartbeat,
-	}
-
-	sc.BaseService = *cmn.NewBaseService(logger, "IPCVal", sc)
-
-	return sc
-}
-
-// OnStart implements cmn.Service.
-func (sc *IPCVal) OnStart() error {
-	err := sc.connect()
-	if err != nil {
-		sc.Logger.Error("OnStart", "err", err)
-		return err
-	}
-
-	sc.RemoteSignerClient, err = NewRemoteSignerClient(sc.conn)
-	if err != nil {
-		return err
-	}
-
-	// Start a routine to keep the connection alive
-	sc.cancelPing = make(chan struct{}, 1)
-	sc.pingTicker = time.NewTicker(sc.connHeartbeat)
-	go func() {
-		for {
-			select {
-			case <-sc.pingTicker.C:
-				err := sc.Ping()
-				if err != nil {
-					sc.Logger.Error("Ping", "err", err)
-				}
-			case <-sc.cancelPing:
-				sc.pingTicker.Stop()
-				return
-			}
-		}
-	}()
-
-	return nil
-}
-
-// OnStop implements cmn.Service.
-func (sc *IPCVal) OnStop() {
-	if sc.cancelPing != nil {
-		close(sc.cancelPing)
-	}
-
-	if sc.conn != nil {
-		if err := sc.conn.Close(); err != nil {
-			sc.Logger.Error("OnStop", "err", err)
-		}
-	}
-}
-
-func (sc *IPCVal) connect() error {
-	la, err := net.ResolveUnixAddr("unix", sc.addr)
-	if err != nil {
-		return err
-	}
-
-	conn, err := net.DialUnix("unix", nil, la)
-	if err != nil {
-		return err
-	}
-
-	sc.conn = newTimeoutConn(conn, sc.connTimeout)
-
-	return nil
-}

privval/ipc_server.go

@@ -1,132 +0,0 @@
-package privval
-
-import (
-	"io"
-	"net"
-	"time"
-
-	cmn "github.com/tendermint/tendermint/libs/common"
-	"github.com/tendermint/tendermint/libs/log"
-	"github.com/tendermint/tendermint/types"
-)
-
-// IPCRemoteSignerOption sets an optional parameter on the IPCRemoteSigner.
-type IPCRemoteSignerOption func(*IPCRemoteSigner)
-
-// IPCRemoteSignerConnDeadline sets the read and write deadline for connections
-// from external signing processes.
-func IPCRemoteSignerConnDeadline(deadline time.Duration) IPCRemoteSignerOption {
-	return func(ss *IPCRemoteSigner) { ss.connDeadline = deadline }
-}
-
-// IPCRemoteSignerConnRetries sets the amount of attempted retries to connect.
-func IPCRemoteSignerConnRetries(retries int) IPCRemoteSignerOption {
-	return func(ss *IPCRemoteSigner) { ss.connRetries = retries }
-}
-
-// IPCRemoteSigner is a RPC implementation of PrivValidator that listens on a unix socket.
-type IPCRemoteSigner struct {
-	cmn.BaseService
-
-	addr         string
-	chainID      string
-	connDeadline time.Duration
-	connRetries  int
-	privVal      types.PrivValidator
-
-	listener *net.UnixListener
-}
-
-// NewIPCRemoteSigner returns an instance of IPCRemoteSigner.
-func NewIPCRemoteSigner(
-	logger log.Logger,
-	chainID, socketAddr string,
-	privVal types.PrivValidator,
-) *IPCRemoteSigner {
-	rs := &IPCRemoteSigner{
-		addr:         socketAddr,
-		chainID:      chainID,
-		connDeadline: time.Second * defaultConnDeadlineSeconds,
-		connRetries:  defaultDialRetries,
-		privVal:      privVal,
-	}
-
-	rs.BaseService = *cmn.NewBaseService(logger, "IPCRemoteSigner", rs)
-
-	return rs
-}
-
-// OnStart implements cmn.Service.
-func (rs *IPCRemoteSigner) OnStart() error {
-	err := rs.listen()
-	if err != nil {
-		err = cmn.ErrorWrap(err, "listen")
-		rs.Logger.Error("OnStart", "err", err)
-		return err
-	}
-
-	go func() {
-		for {
-			conn, err := rs.listener.AcceptUnix()
-			if err != nil {
-				rs.Logger.Error("AcceptUnix", "err", err)
-				return
-			}
-			go rs.handleConnection(conn)
-		}
-	}()
-
-	return nil
-}
-
-// OnStop implements cmn.Service.
-func (rs *IPCRemoteSigner) OnStop() {
-	if rs.listener != nil {
-		if err := rs.listener.Close(); err != nil {
-			rs.Logger.Error("OnStop", "err", cmn.ErrorWrap(err, "closing listener failed"))
-		}
-	}
-}
-
-func (rs *IPCRemoteSigner) listen() error {
-	la, err := net.ResolveUnixAddr("unix", rs.addr)
-	if err != nil {
-		return err
-	}
-
-	rs.listener, err = net.ListenUnix("unix", la)
-
-	return err
-}
-
-func (rs *IPCRemoteSigner) handleConnection(conn net.Conn) {
-	for {
-		if !rs.IsRunning() {
-			return // Ignore error from listener closing.
-		}
-
-		// Reset the connection deadline
-		conn.SetDeadline(time.Now().Add(rs.connDeadline))
-
-		req, err := readMsg(conn)
-		if err != nil {
-			if err != io.EOF {
-				rs.Logger.Error("handleConnection", "err", err)
-			}
-			return
-		}
-
-		res, err := handleRequest(req, rs.chainID, rs.privVal)
-
-		if err != nil {
-			// only log the error; we'll reply with an error in res
-			rs.Logger.Error("handleConnection", "err", err)
-		}
-
-		err = writeMsg(conn, res)
-		if err != nil {
-			rs.Logger.Error("handleConnection", "err", err)
-			return
-		}
-	}
-}

privval/ipc_test.go

@@ -1,147 +0,0 @@
-package privval
-
-import (
-	"io/ioutil"
-	"os"
-	"testing"
-	"time"
-
-	"github.com/stretchr/testify/assert"
-	"github.com/stretchr/testify/require"
-
-	cmn "github.com/tendermint/tendermint/libs/common"
-	"github.com/tendermint/tendermint/libs/log"
-	"github.com/tendermint/tendermint/types"
-)
-
-func TestIPCPVVote(t *testing.T) {
-	var (
-		chainID = cmn.RandStr(12)
-		sc, rs  = testSetupIPCSocketPair(t, chainID, types.NewMockPV())
-
-		ts    = time.Now()
-		vType = types.PrecommitType
-		want  = &types.Vote{Timestamp: ts, Type: vType}
-		have  = &types.Vote{Timestamp: ts, Type: vType}
-	)
-	defer sc.Stop()
-	defer rs.Stop()
-
-	require.NoError(t, rs.privVal.SignVote(chainID, want))
-	require.NoError(t, sc.SignVote(chainID, have))
-	assert.Equal(t, want.Signature, have.Signature)
-}
-
-func TestIPCPVVoteResetDeadline(t *testing.T) {
-	var (
-		chainID = cmn.RandStr(12)
-		sc, rs  = testSetupIPCSocketPair(t, chainID, types.NewMockPV())
-
-		ts    = time.Now()
-		vType = types.PrecommitType
-		want  = &types.Vote{Timestamp: ts, Type: vType}
-		have  = &types.Vote{Timestamp: ts, Type: vType}
-	)
-	defer sc.Stop()
-	defer rs.Stop()
-
-	time.Sleep(3 * time.Millisecond)
-
-	require.NoError(t, rs.privVal.SignVote(chainID, want))
-	require.NoError(t, sc.SignVote(chainID, have))
-	assert.Equal(t, want.Signature, have.Signature)
-
-	// This would exceed the deadline if it was not extended by the previous message
-	time.Sleep(3 * time.Millisecond)
-
-	require.NoError(t, rs.privVal.SignVote(chainID, want))
-	require.NoError(t, sc.SignVote(chainID, have))
-	assert.Equal(t, want.Signature, have.Signature)
-}
-
-func TestIPCPVVoteKeepalive(t *testing.T) {
-	var (
-		chainID = cmn.RandStr(12)
-		sc, rs  = testSetupIPCSocketPair(t, chainID, types.NewMockPV())
-
-		ts    = time.Now()
-		vType = types.PrecommitType
-		want  = &types.Vote{Timestamp: ts, Type: vType}
-		have  = &types.Vote{Timestamp: ts, Type: vType}
-	)
-	defer sc.Stop()
-	defer rs.Stop()
-
-	time.Sleep(10 * time.Millisecond)
-
-	require.NoError(t, rs.privVal.SignVote(chainID, want))
-	require.NoError(t, sc.SignVote(chainID, have))
-	assert.Equal(t, want.Signature, have.Signature)
-}
-
-func testSetupIPCSocketPair(
-	t *testing.T,
-	chainID string,
-	privValidator types.PrivValidator,
-) (*IPCVal, *IPCRemoteSigner) {
-	addr, err := testUnixAddr()
-	require.NoError(t, err)
-
-	var (
-		logger  = log.TestingLogger()
-		privVal = privValidator
-		readyc  = make(chan struct{})
-		rs      = NewIPCRemoteSigner(
-			logger,
-			chainID,
-			addr,
-			privVal,
-		)
-		sc = NewIPCVal(
-			logger,
-			addr,
-		)
-	)
-
-	IPCValConnTimeout(5 * time.Millisecond)(sc)
-	IPCValHeartbeat(time.Millisecond)(sc)
-
-	IPCRemoteSignerConnDeadline(time.Millisecond * 5)(rs)
-
-	testStartIPCRemoteSigner(t, readyc, rs)
-
-	<-readyc
-
-	require.NoError(t, sc.Start())
-	assert.True(t, sc.IsRunning())
-
-	return sc, rs
-}
-
-func testStartIPCRemoteSigner(t *testing.T, readyc chan struct{}, rs *IPCRemoteSigner) {
-	go func(rs *IPCRemoteSigner) {
-		require.NoError(t, rs.Start())
-		assert.True(t, rs.IsRunning())
-
-		readyc <- struct{}{}
-	}(rs)
-}
-
-func testUnixAddr() (string, error) {
-	f, err := ioutil.TempFile("/tmp", "nettest")
-	if err != nil {
-		return "", err
-	}
-
-	addr := f.Name()
-	err = f.Close()
-	if err != nil {
-		return "", err
-	}
-	err = os.Remove(addr)
-	if err != nil {
-		return "", err
-	}
-
-	return addr, nil
-}

privval/remote_signer.go

@@ -4,7 +4,6 @@ import (
 	"fmt"
 	"io"
 	"net"
-	"sync"
 
 	"github.com/pkg/errors"
 
@@ -14,31 +13,41 @@ import (
 	"github.com/tendermint/tendermint/types"
 )
 
-// RemoteSignerClient implements PrivValidator, it uses a socket to request signatures
+// Socket errors.
+var (
+	ErrConnTimeout = errors.New("remote signer timed out")
+)
+
+// RemoteSignerClient implements PrivValidator.
+// It uses a net.Conn to request signatures
 // from an external process.
 type RemoteSignerClient struct {
-	conn            net.Conn
+	conn net.Conn
+
+	// memoized
 	consensusPubKey crypto.PubKey
-	mtx             sync.Mutex
 }
 
 // Check that RemoteSignerClient implements PrivValidator.
 var _ types.PrivValidator = (*RemoteSignerClient)(nil)
 
 // NewRemoteSignerClient returns an instance of RemoteSignerClient.
-func NewRemoteSignerClient(
-	conn net.Conn,
-) (*RemoteSignerClient, error) {
-	sc := &RemoteSignerClient{
-		conn: conn,
-	}
-	pubKey, err := sc.getPubKey()
+func NewRemoteSignerClient(conn net.Conn) (*RemoteSignerClient, error) {
+
+	// retrieve and memoize the consensus public key once.
+	pubKey, err := getPubKey(conn)
 	if err != nil {
 		return nil, cmn.ErrorWrap(err, "error while retrieving public key for remote signer")
 	}
-	// retrieve and memoize the consensus public key once:
-	sc.consensusPubKey = pubKey
-	return sc, nil
+	return &RemoteSignerClient{
+		conn:            conn,
+		consensusPubKey: pubKey,
+	}, nil
+}
+
+// Close calls Close on the underlying net.Conn.
+func (sc *RemoteSignerClient) Close() error {
+	return sc.conn.Close()
 }
 
 // GetPubKey implements PrivValidator.
@@ -46,16 +55,14 @@ func (sc *RemoteSignerClient) GetPubKey() crypto.PubKey {
 	return sc.consensusPubKey
 }
 
-func (sc *RemoteSignerClient) getPubKey() (crypto.PubKey, error) {
-	sc.mtx.Lock()
-	defer sc.mtx.Unlock()
-
-	err := writeMsg(sc.conn, &PubKeyRequest{})
+// not thread-safe (only called on startup).
+func getPubKey(conn net.Conn) (crypto.PubKey, error) {
+	err := writeMsg(conn, &PubKeyRequest{})
 	if err != nil {
 		return nil, err
 	}
 
-	res, err := readMsg(sc.conn)
+	res, err := readMsg(conn)
 	if err != nil {
 		return nil, err
 	}
@@ -73,9 +80,6 @@ func (sc *RemoteSignerClient) getPubKey() (crypto.PubKey, error) {
 
 // SignVote implements PrivValidator.
 func (sc *RemoteSignerClient) SignVote(chainID string, vote *types.Vote) error {
-	sc.mtx.Lock()
-	defer sc.mtx.Unlock()
-
 	err := writeMsg(sc.conn, &SignVoteRequest{Vote: vote})
 	if err != nil {
 		return err
@@ -103,9 +107,6 @@ func (sc *RemoteSignerClient) SignProposal(
 	chainID string,
 	proposal *types.Proposal,
 ) error {
-	sc.mtx.Lock()
-	defer sc.mtx.Unlock()
-
 	err := writeMsg(sc.conn, &SignProposalRequest{Proposal: proposal})
 	if err != nil {
 		return err
@@ -129,9 +130,6 @@ func (sc *RemoteSignerClient) SignProposal(
 
 // Ping is used to check connection health.
 func (sc *RemoteSignerClient) Ping() error {
-	sc.mtx.Lock()
-	defer sc.mtx.Unlock()
-
 	err := writeMsg(sc.conn, &PingRequest{})
 	if err != nil {
 		return err
@@ -260,3 +258,18 @@ func handleRequest(req RemoteSignerMsg, chainID string, privVal types.PrivValida
 
 	return res, err
 }
+
+// IsConnTimeout returns a boolean indicating whether the error is known to
+// report that a connection timeout occurred. This detects both fundamental
+// network timeouts, as well as ErrConnTimeout errors.
+func IsConnTimeout(err error) bool {
+	if cmnErr, ok := err.(cmn.Error); ok {
+		if cmnErr.Data() == ErrConnTimeout {
+			return true
+		}
+	}
+	if _, ok := err.(timeoutError); ok {
+		return true
+	}
+	return false
+}

privval/remote_signer_test.go

@@ -0,0 +1,90 @@
+package privval
+
+import (
+	"net"
+	"testing"
+	"time"
+
+	"github.com/pkg/errors"
+	"github.com/stretchr/testify/assert"
+	"github.com/stretchr/testify/require"
+	"github.com/tendermint/tendermint/crypto/ed25519"
+	cmn "github.com/tendermint/tendermint/libs/common"
+	"github.com/tendermint/tendermint/libs/log"
+	"github.com/tendermint/tendermint/types"
+)
+
+// TestRemoteSignerRetryTCPOnly will test connection retry attempts over TCP. We
+// don't need this for Unix sockets because the OS instantly knows the state of
+// both ends of the socket connection. This basically causes the
+// RemoteSigner.dialer() call inside RemoteSigner.connect() to return
+// successfully immediately, putting an instant stop to any retry attempts.
+func TestRemoteSignerRetryTCPOnly(t *testing.T) {
+	var (
+		attemptc = make(chan int)
+		retries  = 2
+	)
+
+	ln, err := net.Listen("tcp", "127.0.0.1:0")
+	require.NoError(t, err)
+
+	go func(ln net.Listener, attemptc chan<- int) {
+		attempts := 0
+
+		for {
+			conn, err := ln.Accept()
+			require.NoError(t, err)
+
+			err = conn.Close()
+			require.NoError(t, err)
+
+			attempts++
+
+			if attempts == retries {
+				attemptc <- attempts
+				break
+			}
+		}
+	}(ln, attemptc)
+
+	rs := NewRemoteSigner(
+		log.TestingLogger(),
+		cmn.RandStr(12),
+		types.NewMockPV(),
+		DialTCPFn(ln.Addr().String(), testConnDeadline, ed25519.GenPrivKey()),
+	)
+	defer rs.Stop()
+
+	RemoteSignerConnDeadline(time.Millisecond)(rs)
+	RemoteSignerConnRetries(retries)(rs)
+
+	assert.Equal(t, rs.Start(), ErrDialRetryMax)
+
+	select {
+	case attempts := <-attemptc:
+		assert.Equal(t, retries, attempts)
+	case <-time.After(100 * time.Millisecond):
+		t.Error("expected remote to observe connection attempts")
+	}
+}
+
+func TestIsConnTimeoutForFundamentalTimeouts(t *testing.T) {
+	// Generate a networking timeout
+	dialer := DialTCPFn(testFreeTCPAddr(t), time.Millisecond, ed25519.GenPrivKey())
+	_, err := dialer()
+	assert.Error(t, err)
+	assert.True(t, IsConnTimeout(err))
+}
+
+func TestIsConnTimeoutForWrappedConnTimeouts(t *testing.T) {
+	dialer := DialTCPFn(testFreeTCPAddr(t), time.Millisecond, ed25519.GenPrivKey())
+	_, err := dialer()
+	assert.Error(t, err)
+	err = cmn.ErrorWrap(ErrConnTimeout, err.Error())
+	assert.True(t, IsConnTimeout(err))
+}
+
+func TestIsConnTimeoutForNonTimeoutErrors(t *testing.T) {
+	assert.False(t, IsConnTimeout(cmn.ErrorWrap(ErrDialRetryMax, "max retries exceeded")))
+	assert.False(t, IsConnTimeout(errors.New("completely irrelevant error")))
+}

privval/server.go

@@ -5,6 +5,7 @@ import (
 	"net"
 	"time"
 
+	"github.com/pkg/errors"
 	"github.com/tendermint/tendermint/crypto/ed25519"
 	cmn "github.com/tendermint/tendermint/libs/common"
 	"github.com/tendermint/tendermint/libs/log"
@@ -12,6 +13,11 @@ import (
 	"github.com/tendermint/tendermint/types"
 )
 
+// Socket errors.
+var (
+	ErrDialRetryMax = errors.New("dialed maximum retries")
+)
+
 // RemoteSignerOption sets an optional parameter on the RemoteSigner.
 type RemoteSignerOption func(*RemoteSigner)
 
@@ -26,38 +32,64 @@ func RemoteSignerConnRetries(retries int) RemoteSignerOption {
 	return func(ss *RemoteSigner) { ss.connRetries = retries }
 }
 
-// RemoteSigner implements PrivValidator by dialing to a socket.
+// RemoteSigner dials using its dialer and responds to any
+// signature requests using its privVal.
 type RemoteSigner struct {
 	cmn.BaseService
 
-	addr         string
 	chainID      string
 	connDeadline time.Duration
 	connRetries  int
-	privKey      ed25519.PrivKeyEd25519
 	privVal      types.PrivValidator
 
-	conn net.Conn
+	dialer Dialer
+	conn   net.Conn
 }
 
-// NewRemoteSigner returns an instance of RemoteSigner.
+// Dialer dials a remote address and returns a net.Conn or an error.
+type Dialer func() (net.Conn, error)
+
+// DialTCPFn dials the given tcp addr, using the given connTimeout and privKey for the
+// authenticated encryption handshake.
+func DialTCPFn(addr string, connTimeout time.Duration, privKey ed25519.PrivKeyEd25519) Dialer {
+	return func() (net.Conn, error) {
+		conn, err := cmn.Connect(addr)
+		if err == nil {
+			err = conn.SetDeadline(time.Now().Add(connTimeout))
+		}
+		if err == nil {
+			conn, err = p2pconn.MakeSecretConnection(conn, privKey)
+		}
+		return conn, err
+	}
+}
+
+// DialUnixFn dials the given unix socket.
+func DialUnixFn(addr string) Dialer {
+	return func() (net.Conn, error) {
+		unixAddr := &net.UnixAddr{addr, "unix"}
+		return net.DialUnix("unix", nil, unixAddr)
+	}
+}
+
+// NewRemoteSigner return a RemoteSigner that will dial using the given
+// dialer and respond to any signature requests over the connection
+// using the given privVal.
 func NewRemoteSigner(
 	logger log.Logger,
-	chainID, socketAddr string,
+	chainID string,
 	privVal types.PrivValidator,
-	privKey ed25519.PrivKeyEd25519,
+	dialer Dialer,
 ) *RemoteSigner {
 	rs := &RemoteSigner{
-		addr:         socketAddr,
 		chainID:      chainID,
 		connDeadline: time.Second * defaultConnDeadlineSeconds,
 		connRetries:  defaultDialRetries,
-		privKey:      privKey,
 		privVal:      privVal,
+		dialer:       dialer,
 	}
 
 	rs.BaseService = *cmn.NewBaseService(logger, "RemoteSigner", rs)
-
 	return rs
 }
 
@@ -68,6 +100,7 @@ func (rs *RemoteSigner) OnStart() error {
 		rs.Logger.Error("OnStart", "err", err)
 		return err
 	}
+	rs.conn = conn
 
 	go rs.handleConnection(conn)
 
@@ -91,36 +124,11 @@ func (rs *RemoteSigner) connect() (net.Conn, error) {
 		if retries != rs.connRetries {
 			time.Sleep(rs.connDeadline)
 		}
-
-		conn, err := cmn.Connect(rs.addr)
+		conn, err := rs.dialer()
 		if err != nil {
-			rs.Logger.Error(
-				"connect",
-				"addr", rs.addr,
-				"err", err,
-			)
-
+			rs.Logger.Error("dialing", "err", err)
 			continue
 		}
-
-		if err := conn.SetDeadline(time.Now().Add(connTimeout)); err != nil {
-			rs.Logger.Error(
-				"connect",
-				"err", err,
-			)
-			continue
-		}
-
-		conn, err = p2pconn.MakeSecretConnection(conn, rs.privKey)
-		if err != nil {
-			rs.Logger.Error(
-				"connect",
-				"err", err,
-			)
-
-			continue
-		}
-
 		return conn, nil
 	}
 
@@ -139,7 +147,7 @@ func (rs *RemoteSigner) handleConnection(conn net.Conn) {
 		req, err := readMsg(conn)
 		if err != nil {
 			if err != io.EOF {
-				rs.Logger.Error("handleConnection", "err", err)
+				rs.Logger.Error("handleConnection readMsg", "err", err)
 			}
 			return
 		}
@@ -148,12 +156,12 @@ func (rs *RemoteSigner) handleConnection(conn net.Conn) {
 
 		if err != nil {
 			// only log the error; we'll reply with an error in res
-			rs.Logger.Error("handleConnection", "err", err)
+			rs.Logger.Error("handleConnection handleRequest", "err", err)
 		}
 
 		err = writeMsg(conn, res)
 		if err != nil {
-			rs.Logger.Error("handleConnection", "err", err)
+			rs.Logger.Error("handleConnection writeMsg", "err", err)
 			return
 		}
 	}

privval/socket.go

@@ -0,0 +1,184 @@
+package privval
+
+import (
+	"net"
+	"time"
+
+	"github.com/tendermint/tendermint/crypto/ed25519"
+	p2pconn "github.com/tendermint/tendermint/p2p/conn"
+)
+
+const (
+	defaultAcceptDeadlineSeconds = 3
+	defaultConnDeadlineSeconds   = 3
+)
+
+// timeoutError can be used to check if an error returned from the netp package
+// was due to a timeout.
+type timeoutError interface {
+	Timeout() bool
+}
+
+//------------------------------------------------------------------
+// TCP Listener
+
+// TCPListenerOption sets an optional parameter on the tcpListener.
+type TCPListenerOption func(*tcpListener)
+
+// TCPListenerAcceptDeadline sets the deadline for the listener.
+// A zero time value disables the deadline.
+func TCPListenerAcceptDeadline(deadline time.Duration) TCPListenerOption {
+	return func(tl *tcpListener) { tl.acceptDeadline = deadline }
+}
+
+// TCPListenerConnDeadline sets the read and write deadline for connections
+// from external signing processes.
+func TCPListenerConnDeadline(deadline time.Duration) TCPListenerOption {
+	return func(tl *tcpListener) { tl.connDeadline = deadline }
+}
+
+// tcpListener implements net.Listener.
+var _ net.Listener = (*tcpListener)(nil)
+
+// tcpListener wraps a *net.TCPListener to standardise protocol timeouts
+// and potentially other tuning parameters. It also returns encrypted connections.
+type tcpListener struct {
+	*net.TCPListener
+
+	secretConnKey ed25519.PrivKeyEd25519
+
+	acceptDeadline time.Duration
+	connDeadline   time.Duration
+}
+
+// NewTCPListener returns a listener that accepts authenticated encrypted connections
+// using the given secretConnKey and the default timeout values.
+func NewTCPListener(ln net.Listener, secretConnKey ed25519.PrivKeyEd25519) *tcpListener {
+	return &tcpListener{
+		TCPListener:    ln.(*net.TCPListener),
+		secretConnKey:  secretConnKey,
+		acceptDeadline: time.Second * defaultAcceptDeadlineSeconds,
+		connDeadline:   time.Second * defaultConnDeadlineSeconds,
+	}
+}
+
+// Accept implements net.Listener.
+func (ln *tcpListener) Accept() (net.Conn, error) {
+	err := ln.SetDeadline(time.Now().Add(ln.acceptDeadline))
+	if err != nil {
+		return nil, err
+	}
+
+	tc, err := ln.AcceptTCP()
+	if err != nil {
+		return nil, err
+	}
+
+	// Wrap the conn in our timeout and encryption wrappers
+	timeoutConn := newTimeoutConn(tc, ln.connDeadline)
+	secretConn, err := p2pconn.MakeSecretConnection(timeoutConn, ln.secretConnKey)
+	if err != nil {
+		return nil, err
+	}
+
+	return secretConn, nil
+}
+
+//------------------------------------------------------------------
+// Unix Listener
+
+// unixListener implements net.Listener.
+var _ net.Listener = (*unixListener)(nil)
+
+type UnixListenerOption func(*unixListener)
+
+// UnixListenerAcceptDeadline sets the deadline for the listener.
+// A zero time value disables the deadline.
+func UnixListenerAcceptDeadline(deadline time.Duration) UnixListenerOption {
+	return func(ul *unixListener) { ul.acceptDeadline = deadline }
+}
+
+// UnixListenerConnDeadline sets the read and write deadline for connections
+// from external signing processes.
+func UnixListenerConnDeadline(deadline time.Duration) UnixListenerOption {
+	return func(ul *unixListener) { ul.connDeadline = deadline }
+}
+
+// unixListener wraps a *net.UnixListener to standardise protocol timeouts
+// and potentially other tuning parameters. It returns unencrypted connections.
+type unixListener struct {
+	*net.UnixListener
+
+	acceptDeadline time.Duration
+	connDeadline   time.Duration
+}
+
+// NewUnixListener returns a listener that accepts unencrypted connections
+// using the default timeout values.
+func NewUnixListener(ln net.Listener) *unixListener {
+	return &unixListener{
+		UnixListener:   ln.(*net.UnixListener),
+		acceptDeadline: time.Second * defaultAcceptDeadlineSeconds,
+		connDeadline:   time.Second * defaultConnDeadlineSeconds,
+	}
+}
+
+// Accept implements net.Listener.
+func (ln *unixListener) Accept() (net.Conn, error) {
+	err := ln.SetDeadline(time.Now().Add(ln.acceptDeadline))
+	if err != nil {
+		return nil, err
+	}
+
+	tc, err := ln.AcceptUnix()
+	if err != nil {
+		return nil, err
+	}
+
+	// Wrap the conn in our timeout wrapper
+	conn := newTimeoutConn(tc, ln.connDeadline)
+
+	// TODO: wrap in something that authenticates
+	// with a MAC - https://github.com/tendermint/tendermint/issues/3099
+
+	return conn, nil
+}
+
+//------------------------------------------------------------------
+// Connection
+
+// timeoutConn implements net.Conn.
+var _ net.Conn = (*timeoutConn)(nil)
+
+// timeoutConn wraps a net.Conn to standardise protocol timeouts / deadline resets.
+type timeoutConn struct {
+	net.Conn
+
+	connDeadline time.Duration
+}
+
+// newTimeoutConn returns an instance of timeoutConn.
+func newTimeoutConn(
+	conn net.Conn,
+	connDeadline time.Duration) *timeoutConn {
+	return &timeoutConn{
+		conn,
+		connDeadline,
+	}
+}
+
+// Read implements net.Conn.
+func (c timeoutConn) Read(b []byte) (n int, err error) {
+	// Reset deadline
+	c.Conn.SetReadDeadline(time.Now().Add(c.connDeadline))
+
+	return c.Conn.Read(b)
+}
+
+// Write implements net.Conn.
+func (c timeoutConn) Write(b []byte) (n int, err error) {
+	// Reset deadline
+	c.Conn.SetWriteDeadline(time.Now().Add(c.connDeadline))
+
+	return c.Conn.Write(b)
+}

privval/socket_test.go

@@ -0,0 +1,133 @@
+package privval
+
+import (
+	"io/ioutil"
+	"net"
+	"os"
+	"testing"
+	"time"
+
+	"github.com/tendermint/tendermint/crypto/ed25519"
+)
+
+//-------------------------------------------
+// helper funcs
+
+func newPrivKey() ed25519.PrivKeyEd25519 {
+	return ed25519.GenPrivKey()
+}
+
+//-------------------------------------------
+// tests
+
+type listenerTestCase struct {
+	description string // For test reporting purposes.
+	listener    net.Listener
+	dialer      Dialer
+}
+
+// testUnixAddr will attempt to obtain a platform-independent temporary file
+// name for a Unix socket
+func testUnixAddr() (string, error) {
+	f, err := ioutil.TempFile("", "tendermint-privval-test-*")
+	if err != nil {
+		return "", err
+	}
+	addr := f.Name()
+	f.Close()
+	os.Remove(addr)
+	return addr, nil
+}
+
+func tcpListenerTestCase(t *testing.T, acceptDeadline, connectDeadline time.Duration) listenerTestCase {
+	ln, err := net.Listen("tcp", "127.0.0.1:0")
+	if err != nil {
+		t.Fatal(err)
+	}
+
+	tcpLn := NewTCPListener(ln, newPrivKey())
+	TCPListenerAcceptDeadline(acceptDeadline)(tcpLn)
+	TCPListenerConnDeadline(connectDeadline)(tcpLn)
+	return listenerTestCase{
+		description: "TCP",
+		listener:    tcpLn,
+		dialer:      DialTCPFn(ln.Addr().String(), testConnDeadline, newPrivKey()),
+	}
+}
+
+func unixListenerTestCase(t *testing.T, acceptDeadline, connectDeadline time.Duration) listenerTestCase {
+	addr, err := testUnixAddr()
+	if err != nil {
+		t.Fatal(err)
+	}
+	ln, err := net.Listen("unix", addr)
+	if err != nil {
+		t.Fatal(err)
+	}
+
+	unixLn := NewUnixListener(ln)
+	UnixListenerAcceptDeadline(acceptDeadline)(unixLn)
+	UnixListenerConnDeadline(connectDeadline)(unixLn)
+	return listenerTestCase{
+		description: "Unix",
+		listener:    unixLn,
+		dialer:      DialUnixFn(addr),
+	}
+}
+
+func listenerTestCases(t *testing.T, acceptDeadline, connectDeadline time.Duration) []listenerTestCase {
+	return []listenerTestCase{
+		tcpListenerTestCase(t, acceptDeadline, connectDeadline),
+		unixListenerTestCase(t, acceptDeadline, connectDeadline),
+	}
+}
+
+func TestListenerAcceptDeadlines(t *testing.T) {
+	for _, tc := range listenerTestCases(t, time.Millisecond, time.Second) {
+		_, err := tc.listener.Accept()
+		opErr, ok := err.(*net.OpError)
+		if !ok {
+			t.Fatalf("for %s listener, have %v, want *net.OpError", tc.description, err)
+		}
+
+		if have, want := opErr.Op, "accept"; have != want {
+			t.Errorf("for %s listener,  have %v, want %v", tc.description, have, want)
+		}
+	}
+}
+
+func TestListenerConnectDeadlines(t *testing.T) {
+	for _, tc := range listenerTestCases(t, time.Second, time.Millisecond) {
+		readyc := make(chan struct{})
+		donec := make(chan struct{})
+		go func(ln net.Listener) {
+			defer close(donec)
+
+			c, err := ln.Accept()
+			if err != nil {
+				t.Fatal(err)
+			}
+			<-readyc
+
+			time.Sleep(2 * time.Millisecond)
+
+			msg := make([]byte, 200)
+			_, err = c.Read(msg)
+			opErr, ok := err.(*net.OpError)
+			if !ok {
+				t.Fatalf("for %s listener, have %v, want *net.OpError", tc.description, err)
+			}
+
+			if have, want := opErr.Op, "read"; have != want {
+				t.Errorf("for %s listener, have %v, want %v", tc.description, have, want)
+			}
+		}(tc.listener)
+
+		_, err := tc.dialer()
+		if err != nil {
+			t.Fatal(err)
+		}
+		close(readyc)
+		<-donec
+	}
+}

privval/tcp.go

@@ -1,216 +0,0 @@
-package privval
-
-import (
-	"errors"
-	"net"
-	"time"
-
-	"github.com/tendermint/tendermint/crypto/ed25519"
-	cmn "github.com/tendermint/tendermint/libs/common"
-	"github.com/tendermint/tendermint/libs/log"
-	p2pconn "github.com/tendermint/tendermint/p2p/conn"
-	"github.com/tendermint/tendermint/types"
-)
-
-const (
-	defaultAcceptDeadlineSeconds = 3
-	defaultConnDeadlineSeconds   = 3
-	defaultConnHeartBeatSeconds  = 2
-	defaultDialRetries           = 10
-)
-
-// Socket errors.
-var (
-	ErrDialRetryMax       = errors.New("dialed maximum retries")
-	ErrConnTimeout        = errors.New("remote signer timed out")
-	ErrUnexpectedResponse = errors.New("received unexpected response")
-)
-
-var (
-	acceptDeadline = time.Second * defaultAcceptDeadlineSeconds
-	connTimeout    = time.Second * defaultConnDeadlineSeconds
-	connHeartbeat  = time.Second * defaultConnHeartBeatSeconds
-)
-
-// TCPValOption sets an optional parameter on the SocketPV.
-type TCPValOption func(*TCPVal)
-
-// TCPValAcceptDeadline sets the deadline for the TCPVal listener.
-// A zero time value disables the deadline.
-func TCPValAcceptDeadline(deadline time.Duration) TCPValOption {
-	return func(sc *TCPVal) { sc.acceptDeadline = deadline }
-}
-
-// TCPValConnTimeout sets the read and write timeout for connections
-// from external signing processes.
-func TCPValConnTimeout(timeout time.Duration) TCPValOption {
-	return func(sc *TCPVal) { sc.connTimeout = timeout }
-}
-
-// TCPValHeartbeat sets the period on which to check the liveness of the
-// connected Signer connections.
-func TCPValHeartbeat(period time.Duration) TCPValOption {
-	return func(sc *TCPVal) { sc.connHeartbeat = period }
-}
-
-// TCPVal implements PrivValidator, it uses a socket to request signatures
-// from an external process.
-type TCPVal struct {
-	cmn.BaseService
-	*RemoteSignerClient
-
-	addr           string
-	acceptDeadline time.Duration
-	connTimeout    time.Duration
-	connHeartbeat  time.Duration
-	privKey        ed25519.PrivKeyEd25519
-
-	conn       net.Conn
-	listener   net.Listener
-	cancelPing chan struct{}
-	pingTicker *time.Ticker
-}
-
-// Check that TCPVal implements PrivValidator.
-var _ types.PrivValidator = (*TCPVal)(nil)
-
-// NewTCPVal returns an instance of TCPVal.
-func NewTCPVal(
-	logger log.Logger,
-	socketAddr string,
-	privKey ed25519.PrivKeyEd25519,
-) *TCPVal {
-	sc := &TCPVal{
-		addr:           socketAddr,
-		acceptDeadline: acceptDeadline,
-		connTimeout:    connTimeout,
-		connHeartbeat:  connHeartbeat,
-		privKey:        privKey,
-	}
-
-	sc.BaseService = *cmn.NewBaseService(logger, "TCPVal", sc)
-
-	return sc
-}
-
-// OnStart implements cmn.Service.
-func (sc *TCPVal) OnStart() error {
-	if err := sc.listen(); err != nil {
-		sc.Logger.Error("OnStart", "err", err)
-		return err
-	}
-
-	conn, err := sc.waitConnection()
-	if err != nil {
-		sc.Logger.Error("OnStart", "err", err)
-		return err
-	}
-
-	sc.conn = conn
-	sc.RemoteSignerClient, err = NewRemoteSignerClient(sc.conn)
-	if err != nil {
-		return err
-	}
-
-	// Start a routine to keep the connection alive
-	sc.cancelPing = make(chan struct{}, 1)
-	sc.pingTicker = time.NewTicker(sc.connHeartbeat)
-	go func() {
-		for {
-			select {
-			case <-sc.pingTicker.C:
-				err := sc.Ping()
-				if err != nil {
-					sc.Logger.Error(
-						"Ping",
-						"err", err,
-					)
-				}
-			case <-sc.cancelPing:
-				sc.pingTicker.Stop()
-				return
-			}
-		}
-	}()
-
-	return nil
-}
-
-// OnStop implements cmn.Service.
-func (sc *TCPVal) OnStop() {
-	if sc.cancelPing != nil {
-		close(sc.cancelPing)
-	}
-
-	if sc.conn != nil {
-		if err := sc.conn.Close(); err != nil {
-			sc.Logger.Error("OnStop", "err", err)
-		}
-	}
-
-	if sc.listener != nil {
-		if err := sc.listener.Close(); err != nil {
-			sc.Logger.Error("OnStop", "err", err)
-		}
-	}
-}
-
-func (sc *TCPVal) acceptConnection() (net.Conn, error) {
-	conn, err := sc.listener.Accept()
-	if err != nil {
-		if !sc.IsRunning() {
-			return nil, nil // Ignore error from listener closing.
-		}
-		return nil, err
-
-	}
-
-	conn, err = p2pconn.MakeSecretConnection(conn, sc.privKey)
-	if err != nil {
-		return nil, err
-	}
-
-	return conn, nil
-}
-
-func (sc *TCPVal) listen() error {
-	ln, err := net.Listen(cmn.ProtocolAndAddress(sc.addr))
-	if err != nil {
-		return err
-	}
-
-	sc.listener = newTCPTimeoutListener(
-		ln,
-		sc.acceptDeadline,
-		sc.connTimeout,
-		sc.connHeartbeat,
-	)
-
-	return nil
-}
-
-// waitConnection uses the configured wait timeout to error if no external
-// process connects in the time period.
-func (sc *TCPVal) waitConnection() (net.Conn, error) {
-	var (
-		connc = make(chan net.Conn, 1)
-		errc  = make(chan error, 1)
-	)
-
-	go func(connc chan<- net.Conn, errc chan<- error) {
-		conn, err := sc.acceptConnection()
-		if err != nil {
-			errc <- err
-			return
-		}
-
-		connc <- conn
-	}(connc, errc)
-
-	select {
-	case conn := <-connc:
-		return conn, nil
-	case err := <-errc:
-		return nil, err
-	}
-}

privval/tcp_socket.go

@@ -1,90 +0,0 @@
-package privval
-
-import (
-	"net"
-	"time"
-)
-
-// timeoutError can be used to check if an error returned from the netp package
-// was due to a timeout.
-type timeoutError interface {
-	Timeout() bool
-}
-
-// tcpTimeoutListener implements net.Listener.
-var _ net.Listener = (*tcpTimeoutListener)(nil)
-
-// tcpTimeoutListener wraps a *net.TCPListener to standardise protocol timeouts
-// and potentially other tuning parameters.
-type tcpTimeoutListener struct {
-	*net.TCPListener
-
-	acceptDeadline time.Duration
-	connDeadline   time.Duration
-	period         time.Duration
-}
-
-// timeoutConn wraps a net.Conn to standardise protocol timeouts / deadline resets.
-type timeoutConn struct {
-	net.Conn
-
-	connDeadline time.Duration
-}
-
-// newTCPTimeoutListener returns an instance of tcpTimeoutListener.
-func newTCPTimeoutListener(
-	ln net.Listener,
-	acceptDeadline, connDeadline time.Duration,
-	period time.Duration,
-) tcpTimeoutListener {
-	return tcpTimeoutListener{
-		TCPListener:    ln.(*net.TCPListener),
-		acceptDeadline: acceptDeadline,
-		connDeadline:   connDeadline,
-		period:         period,
-	}
-}
-
-// newTimeoutConn returns an instance of newTCPTimeoutConn.
-func newTimeoutConn(
-	conn net.Conn,
-	connDeadline time.Duration) *timeoutConn {
-	return &timeoutConn{
-		conn,
-		connDeadline,
-	}
-}
-
-// Accept implements net.Listener.
-func (ln tcpTimeoutListener) Accept() (net.Conn, error) {
-	err := ln.SetDeadline(time.Now().Add(ln.acceptDeadline))
-	if err != nil {
-		return nil, err
-	}
-
-	tc, err := ln.AcceptTCP()
-	if err != nil {
-		return nil, err
-	}
-
-	// Wrap the conn in our timeout wrapper
-	conn := newTimeoutConn(tc, ln.connDeadline)
-
-	return conn, nil
-}
-
-// Read implements net.Listener.
-func (c timeoutConn) Read(b []byte) (n int, err error) {
-	// Reset deadline
-	c.Conn.SetReadDeadline(time.Now().Add(c.connDeadline))
-
-	return c.Conn.Read(b)
-}
-
-// Write implements net.Listener.
-func (c timeoutConn) Write(b []byte) (n int, err error) {
-	// Reset deadline
-	c.Conn.SetWriteDeadline(time.Now().Add(c.connDeadline))
-
-	return c.Conn.Write(b)
-}

privval/tcp_socket_test.go

@@ -1,65 +0,0 @@
-package privval
-
-import (
-	"net"
-	"testing"
-	"time"
-)
-
-func TestTCPTimeoutListenerAcceptDeadline(t *testing.T) {
-	ln, err := net.Listen("tcp", "127.0.0.1:0")
-	if err != nil {
-		t.Fatal(err)
-	}
-
-	ln = newTCPTimeoutListener(ln, time.Millisecond, time.Second, time.Second)
-
-	_, err = ln.Accept()
-	opErr, ok := err.(*net.OpError)
-	if !ok {
-		t.Fatalf("have %v, want *net.OpError", err)
-	}
-
-	if have, want := opErr.Op, "accept"; have != want {
-		t.Errorf("have %v, want %v", have, want)
-	}
-}
-
-func TestTCPTimeoutListenerConnDeadline(t *testing.T) {
-	ln, err := net.Listen("tcp", "127.0.0.1:0")
-	if err != nil {
-		t.Fatal(err)
-	}
-
-	ln = newTCPTimeoutListener(ln, time.Second, time.Millisecond, time.Second)
-
-	donec := make(chan struct{})
-	go func(ln net.Listener) {
-		defer close(donec)
-
-		c, err := ln.Accept()
-		if err != nil {
-			t.Fatal(err)
-		}
-
-		time.Sleep(2 * time.Millisecond)
-
-		msg := make([]byte, 200)
-		_, err = c.Read(msg)
-		opErr, ok := err.(*net.OpError)
-		if !ok {
-			t.Fatalf("have %v, want *net.OpError", err)
-		}
-
-		if have, want := opErr.Op, "read"; have != want {
-			t.Errorf("have %v, want %v", have, want)
-		}
-	}(ln)
-
-	_, err = net.Dial("tcp", ln.Addr().String())
-	if err != nil {
-		t.Fatal(err)
-	}
-
-	<-donec
-}

privval/tcp_test.go

@@ -1,397 +0,0 @@
-package privval
-
-import (
-	"fmt"
-	"net"
-	"testing"
-	"time"
-
-	"github.com/stretchr/testify/assert"
-	"github.com/stretchr/testify/require"
-
-	"github.com/tendermint/tendermint/crypto/ed25519"
-	cmn "github.com/tendermint/tendermint/libs/common"
-	"github.com/tendermint/tendermint/libs/log"
-
-	p2pconn "github.com/tendermint/tendermint/p2p/conn"
-	"github.com/tendermint/tendermint/types"
-)
-
-func TestSocketPVAddress(t *testing.T) {
-	var (
-		chainID = cmn.RandStr(12)
-		sc, rs  = testSetupSocketPair(t, chainID, types.NewMockPV())
-	)
-	defer sc.Stop()
-	defer rs.Stop()
-
-	serverAddr := rs.privVal.GetPubKey().Address()
-	clientAddr := sc.GetPubKey().Address()
-
-	assert.Equal(t, serverAddr, clientAddr)
-}
-
-func TestSocketPVPubKey(t *testing.T) {
-	var (
-		chainID = cmn.RandStr(12)
-		sc, rs  = testSetupSocketPair(t, chainID, types.NewMockPV())
-	)
-	defer sc.Stop()
-	defer rs.Stop()
-
-	clientKey, err := sc.getPubKey()
-	require.NoError(t, err)
-
-	privvalPubKey := rs.privVal.GetPubKey()
-
-	assert.Equal(t, privvalPubKey, clientKey)
-}
-
-func TestSocketPVProposal(t *testing.T) {
-	var (
-		chainID = cmn.RandStr(12)
-		sc, rs  = testSetupSocketPair(t, chainID, types.NewMockPV())
-
-		ts             = time.Now()
-		privProposal   = &types.Proposal{Timestamp: ts}
-		clientProposal = &types.Proposal{Timestamp: ts}
-	)
-	defer sc.Stop()
-	defer rs.Stop()
-
-	require.NoError(t, rs.privVal.SignProposal(chainID, privProposal))
-	require.NoError(t, sc.SignProposal(chainID, clientProposal))
-	assert.Equal(t, privProposal.Signature, clientProposal.Signature)
-}
-
-func TestSocketPVVote(t *testing.T) {
-	var (
-		chainID = cmn.RandStr(12)
-		sc, rs  = testSetupSocketPair(t, chainID, types.NewMockPV())
-
-		ts    = time.Now()
-		vType = types.PrecommitType
-		want  = &types.Vote{Timestamp: ts, Type: vType}
-		have  = &types.Vote{Timestamp: ts, Type: vType}
-	)
-	defer sc.Stop()
-	defer rs.Stop()
-
-	require.NoError(t, rs.privVal.SignVote(chainID, want))
-	require.NoError(t, sc.SignVote(chainID, have))
-	assert.Equal(t, want.Signature, have.Signature)
-}
-
-func TestSocketPVVoteResetDeadline(t *testing.T) {
-	var (
-		chainID = cmn.RandStr(12)
-		sc, rs  = testSetupSocketPair(t, chainID, types.NewMockPV())
-
-		ts    = time.Now()
-		vType = types.PrecommitType
-		want  = &types.Vote{Timestamp: ts, Type: vType}
-		have  = &types.Vote{Timestamp: ts, Type: vType}
-	)
-	defer sc.Stop()
-	defer rs.Stop()
-
-	time.Sleep(3 * time.Millisecond)
-
-	require.NoError(t, rs.privVal.SignVote(chainID, want))
-	require.NoError(t, sc.SignVote(chainID, have))
-	assert.Equal(t, want.Signature, have.Signature)
-
-	// This would exceed the deadline if it was not extended by the previous message
-	time.Sleep(3 * time.Millisecond)
-
-	require.NoError(t, rs.privVal.SignVote(chainID, want))
-	require.NoError(t, sc.SignVote(chainID, have))
-	assert.Equal(t, want.Signature, have.Signature)
-}
-
-func TestSocketPVVoteKeepalive(t *testing.T) {
-	var (
-		chainID = cmn.RandStr(12)
-		sc, rs  = testSetupSocketPair(t, chainID, types.NewMockPV())
-
-		ts    = time.Now()
-		vType = types.PrecommitType
-		want  = &types.Vote{Timestamp: ts, Type: vType}
-		have  = &types.Vote{Timestamp: ts, Type: vType}
-	)
-	defer sc.Stop()
-	defer rs.Stop()
-
-	time.Sleep(10 * time.Millisecond)
-
-	require.NoError(t, rs.privVal.SignVote(chainID, want))
-	require.NoError(t, sc.SignVote(chainID, have))
-	assert.Equal(t, want.Signature, have.Signature)
-}
-
-func TestSocketPVDeadline(t *testing.T) {
-	var (
-		addr    = testFreeAddr(t)
-		listenc = make(chan struct{})
-		sc      = NewTCPVal(
-			log.TestingLogger(),
-			addr,
-			ed25519.GenPrivKey(),
-		)
-	)
-
-	TCPValConnTimeout(100 * time.Millisecond)(sc)
-
-	go func(sc *TCPVal) {
-		defer close(listenc)
-
-		assert.Equal(t, sc.Start().(cmn.Error).Data(), ErrConnTimeout)
-
-		assert.False(t, sc.IsRunning())
-	}(sc)
-
-	for {
-		conn, err := cmn.Connect(addr)
-		if err != nil {
-			continue
-		}
-
-		_, err = p2pconn.MakeSecretConnection(
-			conn,
-			ed25519.GenPrivKey(),
-		)
-		if err == nil {
-			break
-		}
-	}
-
-	<-listenc
-}
-
-func TestRemoteSignerRetry(t *testing.T) {
-	var (
-		attemptc = make(chan int)
-		retries  = 2
-	)
-
-	ln, err := net.Listen("tcp", "127.0.0.1:0")
-	require.NoError(t, err)
-
-	go func(ln net.Listener, attemptc chan<- int) {
-		attempts := 0
-
-		for {
-			conn, err := ln.Accept()
-			require.NoError(t, err)
-
-			err = conn.Close()
-			require.NoError(t, err)
-
-			attempts++
-
-			if attempts == retries {
-				attemptc <- attempts
-				break
-			}
-		}
-	}(ln, attemptc)
-
-	rs := NewRemoteSigner(
-		log.TestingLogger(),
-		cmn.RandStr(12),
-		ln.Addr().String(),
-		types.NewMockPV(),
-		ed25519.GenPrivKey(),
-	)
-	defer rs.Stop()
-
-	RemoteSignerConnDeadline(time.Millisecond)(rs)
-	RemoteSignerConnRetries(retries)(rs)
-
-	assert.Equal(t, rs.Start(), ErrDialRetryMax)
-
-	select {
-	case attempts := <-attemptc:
-		assert.Equal(t, retries, attempts)
-	case <-time.After(100 * time.Millisecond):
-		t.Error("expected remote to observe connection attempts")
-	}
-}
-
-func TestRemoteSignVoteErrors(t *testing.T) {
-	var (
-		chainID = cmn.RandStr(12)
-		sc, rs  = testSetupSocketPair(t, chainID, types.NewErroringMockPV())
-
-		ts    = time.Now()
-		vType = types.PrecommitType
-		vote  = &types.Vote{Timestamp: ts, Type: vType}
-	)
-	defer sc.Stop()
-	defer rs.Stop()
-
-	err := writeMsg(sc.conn, &SignVoteRequest{Vote: vote})
-	require.NoError(t, err)
-
-	res, err := readMsg(sc.conn)
-	require.NoError(t, err)
-
-	resp := *res.(*SignedVoteResponse)
-	require.NotNil(t, resp.Error)
-	require.Equal(t, resp.Error.Description, types.ErroringMockPVErr.Error())
-
-	err = rs.privVal.SignVote(chainID, vote)
-	require.Error(t, err)
-	err = sc.SignVote(chainID, vote)
-	require.Error(t, err)
-}
-
-func TestRemoteSignProposalErrors(t *testing.T) {
-	var (
-		chainID = cmn.RandStr(12)
-		sc, rs  = testSetupSocketPair(t, chainID, types.NewErroringMockPV())
-
-		ts       = time.Now()
-		proposal = &types.Proposal{Timestamp: ts}
-	)
-	defer sc.Stop()
-	defer rs.Stop()
-
-	err := writeMsg(sc.conn, &SignProposalRequest{Proposal: proposal})
-	require.NoError(t, err)
-
-	res, err := readMsg(sc.conn)
-	require.NoError(t, err)
-
-	resp := *res.(*SignedProposalResponse)
-	require.NotNil(t, resp.Error)
-	require.Equal(t, resp.Error.Description, types.ErroringMockPVErr.Error())
-
-	err = rs.privVal.SignProposal(chainID, proposal)
-	require.Error(t, err)
-
-	err = sc.SignProposal(chainID, proposal)
-	require.Error(t, err)
-}
-
-func TestErrUnexpectedResponse(t *testing.T) {
-	var (
-		addr    = testFreeAddr(t)
-		logger  = log.TestingLogger()
-		chainID = cmn.RandStr(12)
-		readyc  = make(chan struct{})
-		errc    = make(chan error, 1)
-
-		rs = NewRemoteSigner(
-			logger,
-			chainID,
-			addr,
-			types.NewMockPV(),
-			ed25519.GenPrivKey(),
-		)
-		sc = NewTCPVal(
-			logger,
-			addr,
-			ed25519.GenPrivKey(),
-		)
-	)
-
-	testStartSocketPV(t, readyc, sc)
-	defer sc.Stop()
-	RemoteSignerConnDeadline(time.Millisecond)(rs)
-	RemoteSignerConnRetries(100)(rs)
-	// we do not want to Start() the remote signer here and instead use the connection to
-	// reply with intentionally wrong replies below:
-	rsConn, err := rs.connect()
-	defer rsConn.Close()
-	require.NoError(t, err)
-	require.NotNil(t, rsConn)
-	// send over public key to get the remote signer running:
-	go testReadWriteResponse(t, &PubKeyResponse{}, rsConn)
-	<-readyc
-
-	// Proposal:
-	go func(errc chan error) {
-		errc <- sc.SignProposal(chainID, &types.Proposal{})
-	}(errc)
-	// read request and write wrong response:
-	go testReadWriteResponse(t, &SignedVoteResponse{}, rsConn)
-	err = <-errc
-	require.Error(t, err)
-	require.Equal(t, err, ErrUnexpectedResponse)
-
-	// Vote:
-	go func(errc chan error) {
-		errc <- sc.SignVote(chainID, &types.Vote{})
-	}(errc)
-	// read request and write wrong response:
-	go testReadWriteResponse(t, &SignedProposalResponse{}, rsConn)
-	err = <-errc
-	require.Error(t, err)
-	require.Equal(t, err, ErrUnexpectedResponse)
-}
-
-func testSetupSocketPair(
-	t *testing.T,
-	chainID string,
-	privValidator types.PrivValidator,
-) (*TCPVal, *RemoteSigner) {
-	var (
-		addr    = testFreeAddr(t)
-		logger  = log.TestingLogger()
-		privVal = privValidator
-		readyc  = make(chan struct{})
-		rs      = NewRemoteSigner(
-			logger,
-			chainID,
-			addr,
-			privVal,
-			ed25519.GenPrivKey(),
-		)
-		sc = NewTCPVal(
-			logger,
-			addr,
-			ed25519.GenPrivKey(),
-		)
-	)
-
-	TCPValConnTimeout(5 * time.Millisecond)(sc)
-	TCPValHeartbeat(2 * time.Millisecond)(sc)
-	RemoteSignerConnDeadline(5 * time.Millisecond)(rs)
-	RemoteSignerConnRetries(1e6)(rs)
-
-	testStartSocketPV(t, readyc, sc)
-
-	require.NoError(t, rs.Start())
-	assert.True(t, rs.IsRunning())
-
-	<-readyc
-
-	return sc, rs
-}
-
-func testReadWriteResponse(t *testing.T, resp RemoteSignerMsg, rsConn net.Conn) {
-	_, err := readMsg(rsConn)
-	require.NoError(t, err)
-
-	err = writeMsg(rsConn, resp)
-	require.NoError(t, err)
-}
-
-func testStartSocketPV(t *testing.T, readyc chan struct{}, sc *TCPVal) {
-	go func(sc *TCPVal) {
-		require.NoError(t, sc.Start())
-		assert.True(t, sc.IsRunning())
-
-		readyc <- struct{}{}
-	}(sc)
-}
-
-// testFreeAddr claims a free port so we don't block on listener being ready.
-func testFreeAddr(t *testing.T) string {
-	ln, err := net.Listen("tcp", "127.0.0.1:0")
-	require.NoError(t, err)
-	defer ln.Close()
-
-	return fmt.Sprintf("127.0.0.1:%d", ln.Addr().(*net.TCPAddr).Port)
-}

rpc/client/rpc_test.go

@@ -428,5 +428,10 @@ func TestTxSearch(t *testing.T) {
 		if len(result.Txs) == 0 {
 			t.Fatal("expected a lot of transactions")
 		}
+
+		// query a non existing tx with page 1 and txsPerPage 1
+		result, err = c.TxSearch("app.creator='Cosmoshi Neetowoko'", true, 1, 1)
+		require.Nil(t, err, "%+v", err)
+		require.Len(t, result.Txs, 0)
 	}
 }

rpc/core/net.go

@@ -53,6 +53,7 @@ func NetInfo() (*ctypes.ResultNetInfo, error) {
 			NodeInfo:         nodeInfo,
 			IsOutbound:       peer.IsOutbound(),
 			ConnectionStatus: peer.Status(),
+			RemoteIP:         peer.RemoteIP(),
 		})
 	}
 	// TODO: Should we include PersistentPeers and Seeds in here?

rpc/core/pipe.go

@@ -149,8 +149,19 @@ func validatePage(page, perPage, totalCount int) int {
 }
 
 func validatePerPage(perPage int) int {
-	if perPage < 1 || perPage > maxPerPage {
+	if perPage < 1 {
 		return defaultPerPage
+	} else if perPage > maxPerPage {
+		return maxPerPage
 	}
 	return perPage
 }
+
+func validateSkipCount(page, perPage int) int {
+	skipCount := (page - 1) * perPage
+	if skipCount < 0 {
+		return 0
+	}
+
+	return skipCount
+}

rpc/core/pipe_test.go

@@ -47,7 +47,6 @@ func TestPaginationPage(t *testing.T) {
 }
 
 func TestPaginationPerPage(t *testing.T) {
-
 	cases := []struct {
 		totalCount int
 		perPage    int
@@ -59,7 +58,7 @@ func TestPaginationPerPage(t *testing.T) {
 		{5, defaultPerPage, defaultPerPage},
 		{5, maxPerPage - 1, maxPerPage - 1},
 		{5, maxPerPage, maxPerPage},
-		{5, maxPerPage + 1, defaultPerPage},
+		{5, maxPerPage + 1, maxPerPage},
 	}
 
 	for _, c := range cases {

rpc/core/tx.go

@@ -201,10 +201,11 @@ func TxSearch(query string, prove bool, page, perPage int) (*ctypes.ResultTxSear
 	totalCount := len(results)
 	perPage = validatePerPage(perPage)
 	page = validatePage(page, perPage, totalCount)
-	skipCount := (page - 1) * perPage
+	skipCount := validateSkipCount(page, perPage)
 
 	apiResults := make([]*ctypes.ResultTx, cmn.MinInt(perPage, totalCount-skipCount))
 	var proof types.TxProof
+	// if there's no tx in the results array, we don't need to loop through the apiResults array
 	for i := 0; i < len(apiResults); i++ {
 		r := results[skipCount+i]
 		height := r.Height

rpc/core/types/responses.go

@@ -2,6 +2,7 @@ package core_types
 
 import (
 	"encoding/json"
+	"net"
 	"time"
 
 	abci "github.com/tendermint/tendermint/abci/types"
@@ -110,6 +111,7 @@ type Peer struct {
 	NodeInfo         p2p.DefaultNodeInfo  `json:"node_info"`
 	IsOutbound       bool                 `json:"is_outbound"`
 	ConnectionStatus p2p.ConnectionStatus `json:"connection_status"`
+	RemoteIP         net.IP               `json:"remote_ip"`
 }
 
 // Validators for a height

scripts/json2wal/main.go

@@ -45,7 +45,10 @@ func main() {
 	}
 	defer walFile.Close()
 
-	br := bufio.NewReader(f)
+	// the length of tendermint/wal/MsgInfo in the wal.json may exceed the defaultBufSize(4096) of bufio
+	// because of the byte array in BlockPart
+	// leading to unmarshal error: unexpected end of JSON input
+	br := bufio.NewReaderSize(f, 2*types.BlockPartSizeBytes)
 	dec := cs.NewWALEncoder(walFile)
 
 	for {

state/metrics.go

@@ -7,14 +7,26 @@ import (
 	stdprometheus "github.com/prometheus/client_golang/prometheus"
 )
 
-const MetricsSubsystem = "state"
+const (
+	// MetricsSubsystem is a subsystem shared by all metrics exposed by this
+	// package.
+	MetricsSubsystem = "state"
+)
 
+// Metrics contains metrics exposed by this package.
 type Metrics struct {
 	// Time between BeginBlock and EndBlock.
 	BlockProcessingTime metrics.Histogram
 }
 
-func PrometheusMetrics(namespace string) *Metrics {
+// PrometheusMetrics returns Metrics build using Prometheus client library.
+// Optionally, labels can be provided along with their values ("foo",
+// "fooValue").
+func PrometheusMetrics(namespace string, labelsAndValues ...string) *Metrics {
+	labels := []string{}
+	for i := 0; i < len(labelsAndValues); i += 2 {
+		labels = append(labels, labelsAndValues[i])
+	}
 	return &Metrics{
 		BlockProcessingTime: prometheus.NewHistogramFrom(stdprometheus.HistogramOpts{
 			Namespace: namespace,
@@ -22,10 +34,11 @@ func PrometheusMetrics(namespace string) *Metrics {
 			Name:      "block_processing_time",
 			Help:      "Time between BeginBlock and EndBlock in ms.",
 			Buckets:   stdprometheus.LinearBuckets(1, 10, 10),
-		}, []string{}),
+		}, labels).With(labelsAndValues...),
 	}
 }
 
+// NopMetrics returns no-op Metrics.
 func NopMetrics() *Metrics {
 	return &Metrics{
 		BlockProcessingTime: discard.NewHistogram(),

state/state_test.go

@@ -3,6 +3,7 @@ package state
 import (
 	"bytes"
 	"fmt"
+	"math/big"
 	"testing"
 
 	"github.com/stretchr/testify/assert"
@@ -263,6 +264,346 @@ func TestOneValidatorChangesSaveLoad(t *testing.T) {
 	}
 }
 
+func TestProposerPriorityDoesNotGetResetToZero(t *testing.T) {
+	// assert that we preserve accum when calling updateState:
+	// https://github.com/tendermint/tendermint/issues/2718
+	tearDown, _, state := setupTestCase(t)
+	defer tearDown(t)
+	origVotingPower := int64(10)
+	val1PubKey := ed25519.GenPrivKey().PubKey()
+	val1 := &types.Validator{Address: val1PubKey.Address(), PubKey: val1PubKey, VotingPower: origVotingPower}
+
+	state.Validators = types.NewValidatorSet([]*types.Validator{val1})
+	state.NextValidators = state.Validators
+
+	// NewValidatorSet calls IncrementProposerPriority but uses on a copy of val1
+	assert.EqualValues(t, 0, val1.ProposerPriority)
+
+	block := makeBlock(state, state.LastBlockHeight+1)
+	blockID := types.BlockID{block.Hash(), block.MakePartSet(testPartSize).Header()}
+	abciResponses := &ABCIResponses{
+		EndBlock: &abci.ResponseEndBlock{ValidatorUpdates: nil},
+	}
+	validatorUpdates, err := types.PB2TM.ValidatorUpdates(abciResponses.EndBlock.ValidatorUpdates)
+	require.NoError(t, err)
+	updatedState, err := updateState(state, blockID, &block.Header, abciResponses, validatorUpdates)
+	assert.NoError(t, err)
+
+	assert.Equal(t, -origVotingPower, updatedState.NextValidators.Validators[0].ProposerPriority)
+
+	// add a validator
+	val2PubKey := ed25519.GenPrivKey().PubKey()
+	val2VotingPower := int64(100)
+	updateAddVal := abci.ValidatorUpdate{PubKey: types.TM2PB.PubKey(val2PubKey), Power: val2VotingPower}
+	validatorUpdates, err = types.PB2TM.ValidatorUpdates([]abci.ValidatorUpdate{updateAddVal})
+	assert.NoError(t, err)
+	updatedState2, err := updateState(updatedState, blockID, &block.Header, abciResponses, validatorUpdates)
+	assert.NoError(t, err)
+
+	require.Equal(t, len(updatedState2.NextValidators.Validators), 2)
+	_, addedVal2 := updatedState2.NextValidators.GetByAddress(val2PubKey.Address())
+	// adding a validator should not lead to a ProposerPriority equal to zero (unless the combination of averaging and
+	// incrementing would cause so; which is not the case here)
+	totalPowerBefore2 := origVotingPower                                                      // 10
+	wantVal2ProposerPrio := -(totalPowerBefore2 + (totalPowerBefore2 >> 3)) + val2VotingPower // 89
+	avg := (0 + wantVal2ProposerPrio) / 2                                                     // 44
+	wantVal2ProposerPrio -= avg                                                               // 45
+	totalPowerAfter := origVotingPower + val2VotingPower                                      // 110
+	wantVal2ProposerPrio -= totalPowerAfter                                                   // -65
+	assert.Equal(t, wantVal2ProposerPrio, addedVal2.ProposerPriority)                         // not zero == -65
+
+	// Updating a validator does not reset the ProposerPriority to zero:
+	updatedVotingPowVal2 := int64(1)
+	updateVal := abci.ValidatorUpdate{PubKey: types.TM2PB.PubKey(val2PubKey), Power: updatedVotingPowVal2}
+	validatorUpdates, err = types.PB2TM.ValidatorUpdates([]abci.ValidatorUpdate{updateVal})
+	assert.NoError(t, err)
+	updatedState3, err := updateState(updatedState2, blockID, &block.Header, abciResponses, validatorUpdates)
+	assert.NoError(t, err)
+
+	require.Equal(t, len(updatedState3.NextValidators.Validators), 2)
+	_, prevVal1 := updatedState3.Validators.GetByAddress(val1PubKey.Address())
+	_, updatedVal2 := updatedState3.NextValidators.GetByAddress(val2PubKey.Address())
+
+	expectedVal1PrioBeforeAvg := prevVal1.ProposerPriority + prevVal1.VotingPower // -44 + 10 == -34
+	wantVal2ProposerPrio = wantVal2ProposerPrio + updatedVotingPowVal2            // -64
+	avg = (wantVal2ProposerPrio + expectedVal1PrioBeforeAvg) / 2                  // (-64-34)/2 == -49
+	wantVal2ProposerPrio = wantVal2ProposerPrio - avg                             // -15
+	assert.Equal(t, wantVal2ProposerPrio, updatedVal2.ProposerPriority)           // -15
+}
+
+func TestProposerPriorityProposerAlternates(t *testing.T) {
+	// Regression test that would fail if the inner workings of
+	// IncrementProposerPriority change.
+	// Additionally, make sure that same power validators alternate if both
+	// have the same voting power (and the 2nd was added later).
+	tearDown, _, state := setupTestCase(t)
+	defer tearDown(t)
+	origVotinPower := int64(10)
+	val1PubKey := ed25519.GenPrivKey().PubKey()
+	val1 := &types.Validator{Address: val1PubKey.Address(), PubKey: val1PubKey, VotingPower: origVotinPower}
+
+	// reset state validators to above validator
+	state.Validators = types.NewValidatorSet([]*types.Validator{val1})
+	state.NextValidators = state.Validators
+	// we only have one validator:
+	assert.Equal(t, val1PubKey.Address(), state.Validators.Proposer.Address)
+
+	block := makeBlock(state, state.LastBlockHeight+1)
+	blockID := types.BlockID{block.Hash(), block.MakePartSet(testPartSize).Header()}
+	// no updates:
+	abciResponses := &ABCIResponses{
+		EndBlock: &abci.ResponseEndBlock{ValidatorUpdates: nil},
+	}
+	validatorUpdates, err := types.PB2TM.ValidatorUpdates(abciResponses.EndBlock.ValidatorUpdates)
+	require.NoError(t, err)
+
+	updatedState, err := updateState(state, blockID, &block.Header, abciResponses, validatorUpdates)
+	assert.NoError(t, err)
+
+	// 0 + 10 (initial prio) - 10 (avg) - 10 (mostest - total) = -10
+	assert.Equal(t, -origVotinPower, updatedState.NextValidators.Validators[0].ProposerPriority)
+	assert.Equal(t, val1PubKey.Address(), updatedState.NextValidators.Proposer.Address)
+
+	// add a validator with the same voting power as the first
+	val2PubKey := ed25519.GenPrivKey().PubKey()
+	updateAddVal := abci.ValidatorUpdate{PubKey: types.TM2PB.PubKey(val2PubKey), Power: origVotinPower}
+	validatorUpdates, err = types.PB2TM.ValidatorUpdates([]abci.ValidatorUpdate{updateAddVal})
+	assert.NoError(t, err)
+
+	updatedState2, err := updateState(updatedState, blockID, &block.Header, abciResponses, validatorUpdates)
+	assert.NoError(t, err)
+
+	require.Equal(t, len(updatedState2.NextValidators.Validators), 2)
+	assert.Equal(t, updatedState2.Validators, updatedState.NextValidators)
+
+	// val1 will still be proposer as val2 just got added:
+	assert.Equal(t, val1PubKey.Address(), updatedState.NextValidators.Proposer.Address)
+	assert.Equal(t, updatedState2.Validators.Proposer.Address, updatedState2.NextValidators.Proposer.Address)
+	assert.Equal(t, updatedState2.Validators.Proposer.Address, val1PubKey.Address())
+	assert.Equal(t, updatedState2.NextValidators.Proposer.Address, val1PubKey.Address())
+
+	_, updatedVal1 := updatedState2.NextValidators.GetByAddress(val1PubKey.Address())
+	_, oldVal1 := updatedState2.Validators.GetByAddress(val1PubKey.Address())
+	_, updatedVal2 := updatedState2.NextValidators.GetByAddress(val2PubKey.Address())
+
+	totalPower := origVotinPower
+	v2PrioWhenAddedVal2 := -(totalPower + (totalPower >> 3))
+	v2PrioWhenAddedVal2 = v2PrioWhenAddedVal2 + origVotinPower       // -11 + 10 == -1
+	v1PrioWhenAddedVal2 := oldVal1.ProposerPriority + origVotinPower // -10 + 10 == 0
+	// have to express the AVG in big.Ints as -1/2 == -1 in big.Int while -1/2 == 0 in int64
+	avgSum := big.NewInt(0).Add(big.NewInt(v2PrioWhenAddedVal2), big.NewInt(v1PrioWhenAddedVal2))
+	avg := avgSum.Div(avgSum, big.NewInt(2))
+	expectedVal2Prio := v2PrioWhenAddedVal2 - avg.Int64()
+	totalPower = 2 * origVotinPower // 10 + 10
+	expectedVal1Prio := oldVal1.ProposerPriority + origVotinPower - avg.Int64() - totalPower
+	// val1's ProposerPriority story: -10 (see above) + 10 (voting pow) - (-1) (avg) - 20 (total) == -19
+	assert.EqualValues(t, expectedVal1Prio, updatedVal1.ProposerPriority)
+	// val2 prio when added: -(totalVotingPower + (totalVotingPower >> 3)) == -11
+	// -> -11 + 10  (voting power) - (-1) (avg) == 0
+	assert.EqualValues(t, expectedVal2Prio, updatedVal2.ProposerPriority, "unexpected proposer priority for validator: %v", updatedVal2)
+
+	validatorUpdates, err = types.PB2TM.ValidatorUpdates(abciResponses.EndBlock.ValidatorUpdates)
+	require.NoError(t, err)
+
+	updatedState3, err := updateState(updatedState2, blockID, &block.Header, abciResponses, validatorUpdates)
+	assert.NoError(t, err)
+
+	// proposer changes from now on (every iteration) as long as there are no changes in the validator set:
+	assert.NotEqual(t, updatedState3.Validators.Proposer.Address, updatedState3.NextValidators.Proposer.Address)
+
+	assert.Equal(t, updatedState3.Validators, updatedState2.NextValidators)
+	_, updatedVal1 = updatedState3.NextValidators.GetByAddress(val1PubKey.Address())
+	_, oldVal1 = updatedState3.Validators.GetByAddress(val1PubKey.Address())
+	_, updatedVal2 = updatedState3.NextValidators.GetByAddress(val2PubKey.Address())
+	_, oldVal2 := updatedState3.Validators.GetByAddress(val2PubKey.Address())
+
+	// val2 will be proposer:
+	assert.Equal(t, val2PubKey.Address(), updatedState3.NextValidators.Proposer.Address)
+	// check if expected proposer prio is matched:
+
+	avgSum = big.NewInt(oldVal1.ProposerPriority + origVotinPower + oldVal2.ProposerPriority + origVotinPower)
+	avg = avgSum.Div(avgSum, big.NewInt(2))
+	expectedVal1Prio2 := oldVal1.ProposerPriority + origVotinPower - avg.Int64()
+	expectedVal2Prio2 := oldVal2.ProposerPriority + origVotinPower - avg.Int64() - totalPower
+
+	// -19 + 10 - 0 (avg) == -9
+	assert.EqualValues(t, expectedVal1Prio2, updatedVal1.ProposerPriority, "unexpected proposer priority for validator: %v", updatedVal2)
+	// 0 + 10 - 0 (avg) - 20 (total) == -10
+	assert.EqualValues(t, expectedVal2Prio2, updatedVal2.ProposerPriority, "unexpected proposer priority for validator: %v", updatedVal2)
+
+	// no changes in voting power and both validators have same voting power
+	// -> proposers should alternate:
+	oldState := updatedState3
+	for i := 0; i < 1000; i++ {
+		// no validator updates:
+		abciResponses := &ABCIResponses{
+			EndBlock: &abci.ResponseEndBlock{ValidatorUpdates: nil},
+		}
+		validatorUpdates, err = types.PB2TM.ValidatorUpdates(abciResponses.EndBlock.ValidatorUpdates)
+		require.NoError(t, err)
+
+		updatedState, err := updateState(oldState, blockID, &block.Header, abciResponses, validatorUpdates)
+		assert.NoError(t, err)
+		// alternate (and cyclic priorities):
+		assert.NotEqual(t, updatedState.Validators.Proposer.Address, updatedState.NextValidators.Proposer.Address, "iter: %v", i)
+		assert.Equal(t, oldState.Validators.Proposer.Address, updatedState.NextValidators.Proposer.Address, "iter: %v", i)
+
+		_, updatedVal1 = updatedState.NextValidators.GetByAddress(val1PubKey.Address())
+		_, updatedVal2 = updatedState.NextValidators.GetByAddress(val2PubKey.Address())
+
+		if i%2 == 0 {
+			assert.Equal(t, updatedState.Validators.Proposer.Address, val2PubKey.Address())
+			assert.Equal(t, expectedVal1Prio, updatedVal1.ProposerPriority) // -19
+			assert.Equal(t, expectedVal2Prio, updatedVal2.ProposerPriority) // 0
+		} else {
+			assert.Equal(t, updatedState.Validators.Proposer.Address, val1PubKey.Address())
+			assert.Equal(t, expectedVal1Prio2, updatedVal1.ProposerPriority) // -9
+			assert.Equal(t, expectedVal2Prio2, updatedVal2.ProposerPriority) // -10
+		}
+		// update for next iteration:
+		oldState = updatedState
+	}
+}
+
+func TestLargeGenesisValidator(t *testing.T) {
+	tearDown, _, state := setupTestCase(t)
+	defer tearDown(t)
+	// TODO: increase genesis voting power to sth. more  close to MaxTotalVotingPower with changes that
+	// fix with tendermint/issues/2960; currently, the last iteration would take forever though
+	genesisVotingPower := int64(types.MaxTotalVotingPower / 100000000000000)
+	genesisPubKey := ed25519.GenPrivKey().PubKey()
+	// fmt.Println("genesis addr: ", genesisPubKey.Address())
+	genesisVal := &types.Validator{Address: genesisPubKey.Address(), PubKey: genesisPubKey, VotingPower: genesisVotingPower}
+	// reset state validators to above validator
+	state.Validators = types.NewValidatorSet([]*types.Validator{genesisVal})
+	state.NextValidators = state.Validators
+	require.True(t, len(state.Validators.Validators) == 1)
+
+	// update state a few times with no validator updates
+	// asserts that the single validator's ProposerPrio stays the same
+	oldState := state
+	for i := 0; i < 10; i++ {
+		// no updates:
+		abciResponses := &ABCIResponses{
+			EndBlock: &abci.ResponseEndBlock{ValidatorUpdates: nil},
+		}
+		validatorUpdates, err := types.PB2TM.ValidatorUpdates(abciResponses.EndBlock.ValidatorUpdates)
+		require.NoError(t, err)
+
+		block := makeBlock(oldState, oldState.LastBlockHeight+1)
+		blockID := types.BlockID{block.Hash(), block.MakePartSet(testPartSize).Header()}
+
+		updatedState, err := updateState(oldState, blockID, &block.Header, abciResponses, validatorUpdates)
+		// no changes in voting power (ProposerPrio += VotingPower == 0 in 1st round; than shiftByAvg == no-op,
+		// than -Total == -Voting)
+		// -> no change in ProposerPrio (stays -Total == -VotingPower):
+		assert.EqualValues(t, oldState.NextValidators, updatedState.NextValidators)
+		assert.EqualValues(t, -genesisVotingPower, updatedState.NextValidators.Proposer.ProposerPriority)
+
+		oldState = updatedState
+	}
+	// add another validator, do a few iterations (create blocks),
+	// add more validators with same voting power as the 2nd
+	// let the genesis validator "unbond",
+	// see how long it takes until the effect wears off and both begin to alternate
+	// see: https://github.com/tendermint/tendermint/issues/2960
+	firstAddedValPubKey := ed25519.GenPrivKey().PubKey()
+	// fmt.Println("first added addr: ", firstAddedValPubKey.Address())
+	firstAddedValVotingPower := int64(10)
+	firstAddedVal := abci.ValidatorUpdate{PubKey: types.TM2PB.PubKey(firstAddedValPubKey), Power: firstAddedValVotingPower}
+	validatorUpdates, err := types.PB2TM.ValidatorUpdates([]abci.ValidatorUpdate{firstAddedVal})
+	assert.NoError(t, err)
+	abciResponses := &ABCIResponses{
+		EndBlock: &abci.ResponseEndBlock{ValidatorUpdates: []abci.ValidatorUpdate{firstAddedVal}},
+	}
+	block := makeBlock(oldState, oldState.LastBlockHeight+1)
+	blockID := types.BlockID{block.Hash(), block.MakePartSet(testPartSize).Header()}
+	updatedState, err := updateState(oldState, blockID, &block.Header, abciResponses, validatorUpdates)
+
+	lastState := updatedState
+	for i := 0; i < 200; i++ {
+		// no updates:
+		abciResponses := &ABCIResponses{
+			EndBlock: &abci.ResponseEndBlock{ValidatorUpdates: nil},
+		}
+		validatorUpdates, err := types.PB2TM.ValidatorUpdates(abciResponses.EndBlock.ValidatorUpdates)
+		require.NoError(t, err)
+
+		block := makeBlock(lastState, lastState.LastBlockHeight+1)
+		blockID := types.BlockID{block.Hash(), block.MakePartSet(testPartSize).Header()}
+
+		updatedStateInner, err := updateState(lastState, blockID, &block.Header, abciResponses, validatorUpdates)
+		lastState = updatedStateInner
+	}
+	// set state to last state of above iteration
+	state = lastState
+
+	// set oldState to state before above iteration
+	oldState = updatedState
+	_, oldGenesisVal := oldState.NextValidators.GetByAddress(genesisVal.Address)
+	_, newGenesisVal := state.NextValidators.GetByAddress(genesisVal.Address)
+	_, addedOldVal := oldState.NextValidators.GetByAddress(firstAddedValPubKey.Address())
+	_, addedNewVal := state.NextValidators.GetByAddress(firstAddedValPubKey.Address())
+	// expect large negative proposer priority for both (genesis validator decreased, 2nd validator increased):
+	assert.True(t, oldGenesisVal.ProposerPriority > newGenesisVal.ProposerPriority)
+	assert.True(t, addedOldVal.ProposerPriority < addedNewVal.ProposerPriority)
+
+	// add 10 validators with the same voting power as the one added directly after genesis:
+	for i := 0; i < 10; i++ {
+		addedPubKey := ed25519.GenPrivKey().PubKey()
+
+		addedVal := abci.ValidatorUpdate{PubKey: types.TM2PB.PubKey(addedPubKey), Power: firstAddedValVotingPower}
+		validatorUpdates, err := types.PB2TM.ValidatorUpdates([]abci.ValidatorUpdate{addedVal})
+		assert.NoError(t, err)
+
+		abciResponses := &ABCIResponses{
+			EndBlock: &abci.ResponseEndBlock{ValidatorUpdates: []abci.ValidatorUpdate{addedVal}},
+		}
+		block := makeBlock(oldState, oldState.LastBlockHeight+1)
+		blockID := types.BlockID{block.Hash(), block.MakePartSet(testPartSize).Header()}
+		state, err = updateState(state, blockID, &block.Header, abciResponses, validatorUpdates)
+	}
+	require.Equal(t, 10+2, len(state.NextValidators.Validators))
+
+	// remove genesis validator:
+	removeGenesisVal := abci.ValidatorUpdate{PubKey: types.TM2PB.PubKey(genesisPubKey), Power: 0}
+	abciResponses = &ABCIResponses{
+		EndBlock: &abci.ResponseEndBlock{ValidatorUpdates: []abci.ValidatorUpdate{removeGenesisVal}},
+	}
+	block = makeBlock(oldState, oldState.LastBlockHeight+1)
+	blockID = types.BlockID{block.Hash(), block.MakePartSet(testPartSize).Header()}
+	validatorUpdates, err = types.PB2TM.ValidatorUpdates(abciResponses.EndBlock.ValidatorUpdates)
+	require.NoError(t, err)
+	updatedState, err = updateState(state, blockID, &block.Header, abciResponses, validatorUpdates)
+	require.NoError(t, err)
+	// only the first added val (not the genesis val) should be left
+	assert.Equal(t, 11, len(updatedState.NextValidators.Validators))
+
+	// call update state until the effect for the 3rd added validator
+	// being proposer for a long time after the genesis validator left wears off:
+	curState := updatedState
+	count := 0
+	isProposerUnchanged := true
+	for isProposerUnchanged {
+		abciResponses := &ABCIResponses{
+			EndBlock: &abci.ResponseEndBlock{ValidatorUpdates: nil},
+		}
+		validatorUpdates, err = types.PB2TM.ValidatorUpdates(abciResponses.EndBlock.ValidatorUpdates)
+		require.NoError(t, err)
+		block = makeBlock(curState, curState.LastBlockHeight+1)
+		blockID = types.BlockID{block.Hash(), block.MakePartSet(testPartSize).Header()}
+		curState, err = updateState(curState, blockID, &block.Header, abciResponses, validatorUpdates)
+		if !bytes.Equal(curState.Validators.Proposer.Address, curState.NextValidators.Proposer.Address) {
+			isProposerUnchanged = false
+		}
+		count++
+	}
+	// first proposer change happens after this many iters; we probably want to lower this number:
+	// TODO: change with https://github.com/tendermint/tendermint/issues/2960
+	firstProposerChangeExpectedAfter := 438
+	assert.Equal(t, firstProposerChangeExpectedAfter, count)
+}
+
 func TestStoreLoadValidatorsIncrementsProposerPriority(t *testing.T) {
 	const valSetSize = 2
 	tearDown, stateDB, state := setupTestCase(t)

types/block.go

@@ -11,6 +11,7 @@ import (
 
 	"github.com/tendermint/tendermint/crypto"
 	"github.com/tendermint/tendermint/crypto/merkle"
+	"github.com/tendermint/tendermint/crypto/tmhash"
 	cmn "github.com/tendermint/tendermint/libs/common"
 	"github.com/tendermint/tendermint/version"
 )
@@ -788,11 +789,6 @@ type BlockID struct {
 	PartsHeader PartSetHeader `json:"parts"`
 }
 
-// IsZero returns true if this is the BlockID for a nil-block
-func (blockID BlockID) IsZero() bool {
-	return len(blockID.Hash) == 0 && blockID.PartsHeader.IsZero()
-}
-
 // Equals returns true if the BlockID matches the given BlockID
 func (blockID BlockID) Equals(other BlockID) bool {
 	return bytes.Equal(blockID.Hash, other.Hash) &&
@@ -820,6 +816,19 @@ func (blockID BlockID) ValidateBasic() error {
 	return nil
 }
 
+// IsZero returns true if this is the BlockID of a nil block.
+func (blockID BlockID) IsZero() bool {
+	return len(blockID.Hash) == 0 &&
+		blockID.PartsHeader.IsZero()
+}
+
+// IsComplete returns true if this is a valid BlockID of a non-nil block.
+func (blockID BlockID) IsComplete() bool {
+	return len(blockID.Hash) == tmhash.Size &&
+		blockID.PartsHeader.Total > 0 &&
+		len(blockID.PartsHeader.Hash) == tmhash.Size
+}
+
 // String returns a human readable string representation of the BlockID
 func (blockID BlockID) String() string {
 	return fmt.Sprintf(`%v:%v`, blockID.Hash, blockID.PartsHeader)

types/canonical.go

@@ -36,8 +36,8 @@ type CanonicalVote struct {
 	Type      SignedMsgType // type alias for byte
 	Height    int64         `binary:"fixed64"`
 	Round     int64         `binary:"fixed64"`
-	Timestamp time.Time
 	BlockID   CanonicalBlockID
+	Timestamp time.Time
 	ChainID   string
 }
 
@@ -75,8 +75,8 @@ func CanonicalizeVote(chainID string, vote *Vote) CanonicalVote {
 		Type:      vote.Type,
 		Height:    vote.Height,
 		Round:     int64(vote.Round), // cast int->int64 to make amino encode it fixed64 (does not work for int)
-		Timestamp: vote.Timestamp,
 		BlockID:   CanonicalizeBlockID(vote.BlockID),
+		Timestamp: vote.Timestamp,
 		ChainID:   chainID,
 	}
 }

types/evidence_test.go

@@ -62,7 +62,7 @@ func TestEvidence(t *testing.T) {
 		{vote1, makeVote(val, chainID, 0, 10, 3, 1, blockID2), false},    // wrong round
 		{vote1, makeVote(val, chainID, 0, 10, 2, 2, blockID2), false},    // wrong step
 		{vote1, makeVote(val2, chainID, 0, 10, 2, 1, blockID), false},    // wrong validator
-		{vote1, badVote, false},                                          // signed by wrong key
+		{vote1, badVote, false}, // signed by wrong key
 	}
 
 	pubKey := val.GetPubKey()

types/params.go

@@ -34,7 +34,7 @@ type EvidenceParams struct {
 }
 
 // ValidatorParams restrict the public key types validators can use.
-// NOTE: uses ABCI pubkey naming, not Amino routes.
+// NOTE: uses ABCI pubkey naming, not Amino names.
 type ValidatorParams struct {
 	PubKeyTypes []string `json:"pub_key_types"`
 }
@@ -107,7 +107,7 @@ func (params *ConsensusParams) Validate() error {
 	// Check if keyType is a known ABCIPubKeyType
 	for i := 0; i < len(params.Validator.PubKeyTypes); i++ {
 		keyType := params.Validator.PubKeyTypes[i]
-		if _, ok := ABCIPubKeyTypesToAminoRoutes[keyType]; !ok {
+		if _, ok := ABCIPubKeyTypesToAminoNames[keyType]; !ok {
 			return cmn.NewError("params.Validator.PubKeyTypes[%d], %s, is an unknown pubkey type",
 				i, keyType)
 		}

types/part_set.go

@@ -9,7 +9,6 @@ import (
 	"github.com/pkg/errors"
 
 	"github.com/tendermint/tendermint/crypto/merkle"
-	"github.com/tendermint/tendermint/crypto/tmhash"
 	cmn "github.com/tendermint/tendermint/libs/common"
 )
 
@@ -27,16 +26,6 @@ type Part struct {
 	hash []byte
 }
 
-func (part *Part) Hash() []byte {
-	if part.hash != nil {
-		return part.hash
-	}
-	hasher := tmhash.New()
-	hasher.Write(part.Bytes) // nolint: errcheck, gas
-	part.hash = hasher.Sum(nil)
-	return part.hash
-}
-
 // ValidateBasic performs basic validation.
 func (part *Part) ValidateBasic() error {
 	if part.Index < 0 {
@@ -75,7 +64,7 @@ func (psh PartSetHeader) String() string {
 }
 
 func (psh PartSetHeader) IsZero() bool {
-	return psh.Total == 0
+	return psh.Total == 0 && len(psh.Hash) == 0
 }
 
 func (psh PartSetHeader) Equals(other PartSetHeader) bool {
@@ -217,7 +206,7 @@ func (ps *PartSet) AddPart(part *Part) (bool, error) {
 	}
 
 	// Check hash proof
-	if part.Proof.Verify(ps.Hash(), part.Hash()) != nil {
+	if part.Proof.Verify(ps.Hash(), part.Bytes) != nil {
 		return false, ErrPartSetInvalidProof
 	}
 

types/proposal.go

@@ -60,6 +60,10 @@ func (p *Proposal) ValidateBasic() error {
 	if err := p.BlockID.ValidateBasic(); err != nil {
 		return fmt.Errorf("Wrong BlockID: %v", err)
 	}
+	// ValidateBasic above would pass even if the BlockID was empty:
+	if !p.BlockID.IsComplete() {
+		return fmt.Errorf("Expected a complete, non-empty BlockID, got: %v", p.BlockID)
+	}
 
 	// NOTE: Timestamp validation is subtle and handled elsewhere.
 

types/protobuf.go

@@ -25,9 +25,9 @@ const (
 )
 
 // TODO: Make non-global by allowing for registration of more pubkey types
-var ABCIPubKeyTypesToAminoRoutes = map[string]string{
-	ABCIPubKeyTypeEd25519:   ed25519.PubKeyAminoRoute,
-	ABCIPubKeyTypeSecp256k1: secp256k1.PubKeyAminoRoute,
+var ABCIPubKeyTypesToAminoNames = map[string]string{
+	ABCIPubKeyTypeEd25519:   ed25519.PubKeyAminoName,
+	ABCIPubKeyTypeSecp256k1: secp256k1.PubKeyAminoName,
 }
 
 //-------------------------------------------------------

types/results_test.go

@@ -38,7 +38,7 @@ func TestABCIResults(t *testing.T) {
 
 	for i, res := range results {
 		proof := results.ProveResult(i)
-		valid := proof.Verify(root, res.Hash())
+		valid := proof.Verify(root, res.Bytes())
 		assert.NoError(t, valid, "%d", i)
 	}
 }

types/tx.go

@@ -31,13 +31,14 @@ func (tx Tx) String() string {
 // Txs is a slice of Tx.
 type Txs []Tx
 
-// Hash returns the simple Merkle root hash of the transactions.
+// Hash returns the Merkle root hash of the transaction hashes.
+// i.e. the leaves of the tree are the hashes of the txs.
 func (txs Txs) Hash() []byte {
 	// These allocations will be removed once Txs is switched to [][]byte,
 	// ref #2603. This is because golang does not allow type casting slices without unsafe
 	txBzs := make([][]byte, len(txs))
 	for i := 0; i < len(txs); i++ {
-		txBzs[i] = txs[i]
+		txBzs[i] = txs[i].Hash()
 	}
 	return merkle.SimpleHashFromByteSlices(txBzs)
 }
@@ -69,7 +70,7 @@ func (txs Txs) Proof(i int) TxProof {
 	l := len(txs)
 	bzs := make([][]byte, l)
 	for i := 0; i < l; i++ {
-		bzs[i] = txs[i]
+		bzs[i] = txs[i].Hash()
 	}
 	root, proofs := merkle.SimpleProofsFromByteSlices(bzs)
 
@@ -87,8 +88,8 @@ type TxProof struct {
 	Proof    merkle.SimpleProof
 }
 
-// LeadHash returns the hash of the this proof refers to.
-func (tp TxProof) LeafHash() []byte {
+// Leaf returns the hash(tx), which is the leaf in the merkle tree which this proof refers to.
+func (tp TxProof) Leaf() []byte {
 	return tp.Data.Hash()
 }
 
@@ -104,7 +105,7 @@ func (tp TxProof) Validate(dataHash []byte) error {
 	if tp.Proof.Total <= 0 {
 		return errors.New("Proof total must be positive")
 	}
-	valid := tp.Proof.Verify(tp.RootHash, tp.LeafHash())
+	valid := tp.Proof.Verify(tp.RootHash, tp.Leaf())
 	if valid != nil {
 		return errors.New("Proof is not internally consistent")
 	}

types/tx_test.go

@@ -66,14 +66,13 @@ func TestValidTxProof(t *testing.T) {
 		root := txs.Hash()
 		// make sure valid proof for every tx
 		for i := range txs {
-			leaf := txs[i]
-			leafHash := leaf.Hash()
+			tx := []byte(txs[i])
 			proof := txs.Proof(i)
 			assert.Equal(t, i, proof.Proof.Index, "%d: %d", h, i)
 			assert.Equal(t, len(txs), proof.Proof.Total, "%d: %d", h, i)
 			assert.EqualValues(t, root, proof.RootHash, "%d: %d", h, i)
-			assert.EqualValues(t, leaf, proof.Data, "%d: %d", h, i)
-			assert.EqualValues(t, leafHash, proof.LeafHash(), "%d: %d", h, i)
+			assert.EqualValues(t, tx, proof.Data, "%d: %d", h, i)
+			assert.EqualValues(t, txs[i].Hash(), proof.Leaf(), "%d: %d", h, i)
 			assert.Nil(t, proof.Validate(root), "%d: %d", h, i)
 			assert.NotNil(t, proof.Validate([]byte("foobar")), "%d: %d", h, i)
 

types/vote.go

@@ -52,8 +52,8 @@ type Vote struct {
 	Type             SignedMsgType `json:"type"`
 	Height           int64         `json:"height"`
 	Round            int           `json:"round"`
-	Timestamp        time.Time     `json:"timestamp"`
 	BlockID          BlockID       `json:"block_id"` // zero if vote is nil.
+	Timestamp        time.Time     `json:"timestamp"`
 	ValidatorAddress Address       `json:"validator_address"`
 	ValidatorIndex   int           `json:"validator_index"`
 	Signature        []byte        `json:"signature"`
@@ -127,6 +127,11 @@ func (vote *Vote) ValidateBasic() error {
 	if err := vote.BlockID.ValidateBasic(); err != nil {
 		return fmt.Errorf("Wrong BlockID: %v", err)
 	}
+	// BlockID.ValidateBasic would not err if we for instance have an empty hash but a
+	// non-empty PartsSetHeader:
+	if !vote.BlockID.IsZero() && !vote.BlockID.IsComplete() {
+		return fmt.Errorf("BlockID must be either empty or complete, got: %v", vote.BlockID)
+	}
 	if len(vote.ValidatorAddress) != crypto.AddressSize {
 		return fmt.Errorf("Expected ValidatorAddress size to be %d bytes, got %d bytes",
 			crypto.AddressSize,

types/vote_test.go

@@ -63,8 +63,8 @@ func TestVoteSignableTestVectors(t *testing.T) {
 		{
 			CanonicalizeVote("", &Vote{}),
 			// NOTE: Height and Round are skipped here. This case needs to be considered while parsing.
-			// []byte{0x22, 0x9, 0x9, 0x0, 0x9, 0x6e, 0x88, 0xf1, 0xff, 0xff, 0xff},
-			[]byte{0x22, 0xb, 0x8, 0x80, 0x92, 0xb8, 0xc3, 0x98, 0xfe, 0xff, 0xff, 0xff, 0x1},
+			// []byte{0x2a, 0x9, 0x9, 0x0, 0x9, 0x6e, 0x88, 0xf1, 0xff, 0xff, 0xff},
+			[]byte{0x2a, 0xb, 0x8, 0x80, 0x92, 0xb8, 0xc3, 0x98, 0xfe, 0xff, 0xff, 0xff, 0x1},
 		},
 		// with proper (fixed size) height and round (PreCommit):
 		{
@@ -76,7 +76,7 @@ func TestVoteSignableTestVectors(t *testing.T) {
 				0x1, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, // height
 				0x19,                                   // (field_number << 3) | wire_type
 				0x1, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, // round
-				0x22, // (field_number << 3) | wire_type
+				0x2a, // (field_number << 3) | wire_type
 				// remaining fields (timestamp):
 				0xb, 0x8, 0x80, 0x92, 0xb8, 0xc3, 0x98, 0xfe, 0xff, 0xff, 0xff, 0x1},
 		},
@@ -90,7 +90,7 @@ func TestVoteSignableTestVectors(t *testing.T) {
 				0x1, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, // height
 				0x19,                                   // (field_number << 3) | wire_type
 				0x1, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, // round
-				0x22, // (field_number << 3) | wire_type
+				0x2a, // (field_number << 3) | wire_type
 				// remaining fields (timestamp):
 				0xb, 0x8, 0x80, 0x92, 0xb8, 0xc3, 0x98, 0xfe, 0xff, 0xff, 0xff, 0x1},
 		},
@@ -102,7 +102,7 @@ func TestVoteSignableTestVectors(t *testing.T) {
 				0x19,                                   // (field_number << 3) | wire_type
 				0x1, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, // round
 				// remaining fields (timestamp):
-				0x22,
+				0x2a,
 				0xb, 0x8, 0x80, 0x92, 0xb8, 0xc3, 0x98, 0xfe, 0xff, 0xff, 0xff, 0x1},
 		},
 		// containing non-empty chain_id:
@@ -114,7 +114,7 @@ func TestVoteSignableTestVectors(t *testing.T) {
 				0x19,                                   // (field_number << 3) | wire_type
 				0x1, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, // round
 				// remaining fields:
-				0x22,                                                                // (field_number << 3) | wire_type
+				0x2a,                                                                // (field_number << 3) | wire_type
 				0xb, 0x8, 0x80, 0x92, 0xb8, 0xc3, 0x98, 0xfe, 0xff, 0xff, 0xff, 0x1, // timestamp
 				0x32,                                                                               // (field_number << 3) | wire_type
 				0xd, 0x74, 0x65, 0x73, 0x74, 0x5f, 0x63, 0x68, 0x61, 0x69, 0x6e, 0x5f, 0x69, 0x64}, // chainID

version/version.go

@@ -18,7 +18,7 @@ const (
 	// TMCoreSemVer is the current version of Tendermint Core.
 	// It's the Semantic Version of the software.
 	// Must be a string because scripts like dist.sh read this file.
-	TMCoreSemVer = "0.27.4"
+	TMCoreSemVer = "0.28.0"
 
 	// ABCISemVer is the semantic version of the ABCI library
 	ABCISemVer  = "0.15.0"