From 0124593a61c12e8614e04d7eaaf724c51ed724b1 Mon Sep 17 00:00:00 2001 From: Sam Kleinman Date: Fri, 26 Mar 2021 15:15:45 -0400 Subject: [PATCH 01/24] fix: avoid race with a deeper copy (#6285) --- consensus/peer_state.go | 2 +- consensus/types/peer_round_state.go | 25 ++++++++++++++++++++ consensus/types/peer_round_state_test.go | 29 ++++++++++++++++++++++++ libs/bits/bit_array.go | 18 ++++++++++----- libs/bits/bit_array_test.go | 2 +- 5 files changed, 68 insertions(+), 8 deletions(-) create mode 100644 consensus/types/peer_round_state_test.go diff --git a/consensus/peer_state.go b/consensus/peer_state.go index 31406a025..1fb16d1be 100644 --- a/consensus/peer_state.go +++ b/consensus/peer_state.go @@ -89,7 +89,7 @@ func (ps *PeerState) GetRoundState() *cstypes.PeerRoundState { ps.mtx.Lock() defer ps.mtx.Unlock() - prs := ps.PRS // copy + prs := ps.PRS.Copy() return &prs } diff --git a/consensus/types/peer_round_state.go b/consensus/types/peer_round_state.go index 07283c5b4..9d294d9af 100644 --- a/consensus/types/peer_round_state.go +++ b/consensus/types/peer_round_state.go @@ -46,6 +46,31 @@ func (prs PeerRoundState) String() string { return prs.StringIndented("") } +// Copy provides a deep copy operation. Because many of the fields in +// the PeerRound struct are pointers, we need an explicit deep copy +// operation to avoid a non-obvious shared data situation. +func (prs PeerRoundState) Copy() PeerRoundState { + // this works because it's not a pointer receiver so it's + // already, effectively a copy. + + headerHash := prs.ProposalBlockPartSetHeader.Hash.Bytes() + + hashCopy := make([]byte, len(headerHash)) + copy(hashCopy, headerHash) + prs.ProposalBlockPartSetHeader = types.PartSetHeader{ + Total: prs.ProposalBlockPartSetHeader.Total, + Hash: hashCopy, + } + prs.ProposalBlockParts = prs.ProposalBlockParts.Copy() + prs.ProposalPOL = prs.ProposalPOL.Copy() + prs.Prevotes = prs.Prevotes.Copy() + prs.Precommits = prs.Precommits.Copy() + prs.LastCommit = prs.LastCommit.Copy() + prs.CatchupCommit = prs.CatchupCommit.Copy() + + return prs +} + // StringIndented returns a string representation of the PeerRoundState func (prs PeerRoundState) StringIndented(indent string) string { return fmt.Sprintf(`PeerRoundState{ diff --git a/consensus/types/peer_round_state_test.go b/consensus/types/peer_round_state_test.go new file mode 100644 index 000000000..393fd2056 --- /dev/null +++ b/consensus/types/peer_round_state_test.go @@ -0,0 +1,29 @@ +package types + +import ( + "testing" + + "github.com/stretchr/testify/require" + "github.com/tendermint/tendermint/libs/bits" +) + +func TestCopy(t *testing.T) { + t.Run("VerifyShallowCopy", func(t *testing.T) { + prsOne := PeerRoundState{} + prsOne.Prevotes = bits.NewBitArray(12) + prsTwo := prsOne + + prsOne.Prevotes.SetIndex(1, true) + + require.Equal(t, prsOne.Prevotes, prsTwo.Prevotes) + }) + t.Run("DeepCopy", func(t *testing.T) { + prsOne := PeerRoundState{} + prsOne.Prevotes = bits.NewBitArray(12) + prsTwo := prsOne.Copy() + + prsOne.Prevotes.SetIndex(1, true) + + require.NotEqual(t, prsOne.Prevotes, prsTwo.Prevotes) + }) +} diff --git a/libs/bits/bit_array.go b/libs/bits/bit_array.go index 1a41d87f9..3ebad38ce 100644 --- a/libs/bits/bit_array.go +++ b/libs/bits/bit_array.go @@ -422,21 +422,21 @@ func (bA *BitArray) UnmarshalJSON(bz []byte) error { // ToProto converts BitArray to protobuf. It returns nil if BitArray is // nil/empty. -// -// XXX: It does not copy the array. func (bA *BitArray) ToProto() *tmprotobits.BitArray { if bA == nil || (len(bA.Elems) == 0 && bA.Bits == 0) { // empty return nil } - return &tmprotobits.BitArray{Bits: int64(bA.Bits), Elems: bA.Elems} + bA.mtx.Lock() + defer bA.mtx.Unlock() + + bc := bA.copy() + return &tmprotobits.BitArray{Bits: int64(bc.Bits), Elems: bc.Elems} } // FromProto sets BitArray to the given protoBitArray. It returns an error if // protoBitArray is invalid. -// -// XXX: It does not copy the array. func (bA *BitArray) FromProto(protoBitArray *tmprotobits.BitArray) error { if protoBitArray == nil { return nil @@ -454,8 +454,14 @@ func (bA *BitArray) FromProto(protoBitArray *tmprotobits.BitArray) error { return fmt.Errorf("invalid number of Elems: got %d, but exp %d", got, exp) } + bA.mtx.Lock() + defer bA.mtx.Unlock() + + ec := make([]uint64, len(protoBitArray.Elems)) + copy(ec, protoBitArray.Elems) + bA.Bits = int(protoBitArray.Bits) - bA.Elems = protoBitArray.Elems + bA.Elems = ec return nil } diff --git a/libs/bits/bit_array_test.go b/libs/bits/bit_array_test.go index 10d607ef2..96f2e2257 100644 --- a/libs/bits/bit_array_test.go +++ b/libs/bits/bit_array_test.go @@ -299,7 +299,7 @@ func TestBitArrayFromProto(t *testing.T) { expErr bool }{ 0: {nil, &BitArray{}, false}, - 1: {&tmprotobits.BitArray{}, &BitArray{}, false}, + 1: {&tmprotobits.BitArray{}, &BitArray{Elems: []uint64{}}, false}, 2: {&tmprotobits.BitArray{Bits: 1, Elems: make([]uint64, 1)}, &BitArray{Bits: 1, Elems: make([]uint64, 1)}, false}, From ca080b5a7f5d3a7ea1d2567a6c3df932ce7db137 Mon Sep 17 00:00:00 2001 From: "dependabot[bot]" <49699333+dependabot[bot]@users.noreply.github.com> Date: Fri, 26 Mar 2021 15:43:30 -0400 Subject: [PATCH 02/24] build(deps): Bump google.golang.org/grpc from 1.36.0 to 1.36.1 (#6281) --- go.mod | 2 +- go.sum | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/go.mod b/go.mod index ae7e6738c..6d366dfed 100644 --- a/go.mod +++ b/go.mod @@ -35,5 +35,5 @@ require ( github.com/tendermint/tm-db v0.6.4 golang.org/x/crypto v0.0.0-20201117144127-c1f2f97bffc9 golang.org/x/net v0.0.0-20201021035429-f5854403a974 - google.golang.org/grpc v1.36.0 + google.golang.org/grpc v1.36.1 ) diff --git a/go.sum b/go.sum index 5d7a3346b..02cf33278 100644 --- a/go.sum +++ b/go.sum @@ -739,8 +739,8 @@ google.golang.org/grpc v1.32.0/go.mod h1:N36X2cJ7JwdamYAgDz+s+rVMFjt3numwzf/HckM google.golang.org/grpc v1.33.1/go.mod h1:fr5YgcSWrqhRRxogOsw7RzIpsmvOZ6IcH4kBYTpR3n0= google.golang.org/grpc v1.33.2/go.mod h1:JMHMWHQWaTccqQQlmk3MJZS+GWXOdAesneDmEnv2fbc= google.golang.org/grpc v1.35.0/go.mod h1:qjiiYl8FncCW8feJPdyg3v6XW24KsRHe+dy9BAGRRjU= -google.golang.org/grpc v1.36.0 h1:o1bcQ6imQMIOpdrO3SWf2z5RV72WbDwdXuK0MDlc8As= -google.golang.org/grpc v1.36.0/go.mod h1:qjiiYl8FncCW8feJPdyg3v6XW24KsRHe+dy9BAGRRjU= +google.golang.org/grpc v1.36.1 h1:cmUfbeGKnz9+2DD/UYsMQXeqbHZqZDs4eQwW0sFOpBY= +google.golang.org/grpc v1.36.1/go.mod h1:qjiiYl8FncCW8feJPdyg3v6XW24KsRHe+dy9BAGRRjU= google.golang.org/protobuf v0.0.0-20200109180630-ec00e32a8dfd/go.mod h1:DFci5gLYBciE7Vtevhsrf46CRTquxDuWsQurQQe4oz8= google.golang.org/protobuf v0.0.0-20200221191635-4d8936d0db64/go.mod h1:kwYJMbMJ01Woi6D6+Kah6886xMZcty6N08ah7+eCXa0= google.golang.org/protobuf v0.0.0-20200228230310-ab0ca4ff8a60/go.mod h1:cfTl7dwQJ+fmap5saPgwCLgHXTUD7jkjRqWcaiX5VyM= From d988cd61018a7ae3884762d4e5188df1c67833c1 Mon Sep 17 00:00:00 2001 From: "dependabot[bot]" <49699333+dependabot[bot]@users.noreply.github.com> Date: Fri, 26 Mar 2021 19:53:56 +0000 Subject: [PATCH 03/24] build(deps): Bump github.com/minio/highwayhash from 1.0.1 to 1.0.2 (#6280) Bumps [github.com/minio/highwayhash](https://github.com/minio/highwayhash) from 1.0.1 to 1.0.2.
Release notes

Sourced from github.com/minio/highwayhash's releases.

Version v1.0.2

Changelog

Fixed

Issue #17 - on arm64 (on Go 1.16) wrong hash values got computed due to incorrectly naming asm constants like regular Go functions. This probably confused the linker and caused the arm64 implementation to compute incorrect hash values. Fixed by 08ce0b4

Commits

[![Dependabot compatibility score](https://dependabot-badges.githubapp.com/badges/compatibility_score?dependency-name=github.com/minio/highwayhash&package-manager=go_modules&previous-version=1.0.1&new-version=1.0.2)](https://docs.github.com/en/github/managing-security-vulnerabilities/about-dependabot-security-updates#about-compatibility-scores) Dependabot will resolve any conflicts with this PR as long as you don't alter it yourself. You can also trigger a rebase manually by commenting `@dependabot rebase`. [//]: # (dependabot-automerge-start) [//]: # (dependabot-automerge-end) ---
Dependabot commands and options
You can trigger Dependabot actions by commenting on this PR: - `@dependabot rebase` will rebase this PR - `@dependabot recreate` will recreate this PR, overwriting any edits that have been made to it - `@dependabot merge` will merge this PR after your CI passes on it - `@dependabot squash and merge` will squash and merge this PR after your CI passes on it - `@dependabot cancel merge` will cancel a previously requested merge and block automerging - `@dependabot reopen` will reopen this PR if it is closed - `@dependabot close` will close this PR and stop Dependabot recreating it. You can achieve the same result by closing it manually - `@dependabot ignore this major version` will close this PR and stop Dependabot creating any more for this major version (unless you reopen the PR or upgrade to it yourself) - `@dependabot ignore this minor version` will close this PR and stop Dependabot creating any more for this minor version (unless you reopen the PR or upgrade to it yourself) - `@dependabot ignore this dependency` will close this PR and stop Dependabot creating any more for this dependency (unless you reopen the PR or upgrade to it yourself)
--- go.mod | 2 +- go.sum | 3 ++- 2 files changed, 3 insertions(+), 2 deletions(-) diff --git a/go.mod b/go.mod index 6d366dfed..1afc2d4a5 100644 --- a/go.mod +++ b/go.mod @@ -22,7 +22,7 @@ require ( github.com/gtank/merlin v0.1.1 github.com/hdevalence/ed25519consensus v0.0.0-20210204194344-59a8610d2b87 github.com/libp2p/go-buffer-pool v0.0.2 - github.com/minio/highwayhash v1.0.1 + github.com/minio/highwayhash v1.0.2 github.com/pkg/errors v0.9.1 github.com/prometheus/client_golang v1.10.0 github.com/rcrowley/go-metrics v0.0.0-20200313005456-10cdbea86bc0 diff --git a/go.sum b/go.sum index 02cf33278..46ca7c349 100644 --- a/go.sum +++ b/go.sum @@ -318,8 +318,9 @@ github.com/matttproud/golang_protobuf_extensions v1.0.1/go.mod h1:D8He9yQNgCq6Z5 github.com/miekg/dns v1.0.14/go.mod h1:W1PPwlIAgtquWBMBEV9nkV9Cazfe8ScdGz/Lj7v3Nrg= github.com/mimoo/StrobeGo v0.0.0-20181016162300-f8f6d4d2b643 h1:hLDRPB66XQT/8+wG9WsDpiCvZf1yKO7sz7scAjSlBa0= github.com/mimoo/StrobeGo v0.0.0-20181016162300-f8f6d4d2b643/go.mod h1:43+3pMjjKimDBf5Kr4ZFNGbLql1zKkbImw+fZbw3geM= -github.com/minio/highwayhash v1.0.1 h1:dZ6IIu8Z14VlC0VpfKofAhCy74wu/Qb5gcn52yWoz/0= github.com/minio/highwayhash v1.0.1/go.mod h1:BQskDq+xkJ12lmlUUi7U0M5Swg3EWR+dLTk+kldvVxY= +github.com/minio/highwayhash v1.0.2 h1:Aak5U0nElisjDCfPSG79Tgzkn2gl66NxOMspRrKnA/g= +github.com/minio/highwayhash v1.0.2/go.mod h1:BQskDq+xkJ12lmlUUi7U0M5Swg3EWR+dLTk+kldvVxY= github.com/mitchellh/cli v1.0.0/go.mod h1:hNIlj7HEI86fIcpObd7a0FcrxTWetlwJDGcceTlRvqc= github.com/mitchellh/go-homedir v1.0.0/go.mod h1:SfyaCUpYCn1Vlf4IUYiD9fPX4A5wJrkLzIz1N1q0pr0= github.com/mitchellh/go-homedir v1.1.0/go.mod h1:SfyaCUpYCn1Vlf4IUYiD9fPX4A5wJrkLzIz1N1q0pr0= From 63f6c260721c45e9bfe62f93ebb5ae189bec1745 Mon Sep 17 00:00:00 2001 From: Callum Waters Date: Mon, 29 Mar 2021 11:13:24 +0200 Subject: [PATCH 04/24] update tm command from node to start (#6283) --- DOCKER/README.md | 2 +- docs/app-dev/getting-started.md | 8 ++++---- docs/architecture/adr-052-tendermint-mode.md | 2 +- docs/introduction/install.md | 2 +- docs/introduction/quick-start.md | 10 +++++----- docs/tendermint-core/using-tendermint.md | 16 ++++++++-------- 6 files changed, 20 insertions(+), 20 deletions(-) diff --git a/DOCKER/README.md b/DOCKER/README.md index 2793fd0f6..2529f3282 100644 --- a/DOCKER/README.md +++ b/DOCKER/README.md @@ -32,7 +32,7 @@ A quick example of a built-in app and Tendermint core in one container. ```sh docker run -it --rm -v "/tmp:/tendermint" tendermint/tendermint init -docker run -it --rm -v "/tmp:/tendermint" tendermint/tendermint node --proxy-app=kvstore +docker run -it --rm -v "/tmp:/tendermint" tendermint/tendermint start --proxy-app=kvstore ``` ## Local cluster diff --git a/docs/app-dev/getting-started.md b/docs/app-dev/getting-started.md index 9ba6bab26..126da9371 100644 --- a/docs/app-dev/getting-started.md +++ b/docs/app-dev/getting-started.md @@ -64,12 +64,12 @@ before, use: ```sh tendermint init -tendermint node +tendermint start ``` If you have used Tendermint, you may want to reset the data for a new blockchain by running `tendermint unsafe_reset_all`. Then you can run -`tendermint node` to start Tendermint, and connect to the app. For more +`tendermint start` to start Tendermint, and connect to the app. For more details, see [the guide on using Tendermint](../tendermint-core/using-tendermint.md). You should see Tendermint making blocks! We can get the status of our @@ -202,7 +202,7 @@ In another window, reset then start Tendermint: ```sh tendermint unsafe_reset_all -tendermint node +tendermint start ``` Once again, you can see the blocks streaming by. Let's send some @@ -277,7 +277,7 @@ In another window, reset and start `tendermint`: ```sh tendermint unsafe_reset_all -tendermint node +tendermint start ``` Once again, you should see blocks streaming by - but now, our diff --git a/docs/architecture/adr-052-tendermint-mode.md b/docs/architecture/adr-052-tendermint-mode.md index 344c68a5b..4848b5356 100644 --- a/docs/architecture/adr-052-tendermint-mode.md +++ b/docs/architecture/adr-052-tendermint-mode.md @@ -45,7 +45,7 @@ We would like to suggest a simple Tendermint mode abstraction. These modes will - Configuration, cli command - We would like to suggest by introducing `mode` parameter in `config.toml` and cli - `mode = "{{ .BaseConfig.Mode }}"` in `config.toml` - - `tendermint node --mode validator` in cli + - `tendermint start --mode validator` in cli - full | validator | seednode (default: "full") - RPC modification - `host:26657/status` diff --git a/docs/introduction/install.md b/docs/introduction/install.md index a08182f51..4d72cb90c 100644 --- a/docs/introduction/install.md +++ b/docs/introduction/install.md @@ -57,7 +57,7 @@ To start a one-node blockchain with a simple in-process application: ```sh tendermint init -tendermint node --proxy-app=kvstore +tendermint start --proxy-app=kvstore ``` ## Reinstall diff --git a/docs/introduction/quick-start.md b/docs/introduction/quick-start.md index 96d96a81c..c1a81c075 100644 --- a/docs/introduction/quick-start.md +++ b/docs/introduction/quick-start.md @@ -59,7 +59,7 @@ Configuring a cluster is covered further below. Start Tendermint with a simple in-process application: ```sh -tendermint node --proxy-app=kvstore +tendermint start --proxy-app=kvstore ``` > Note: `kvstore` is a non persistent app, if you would like to run an application with persistence run `--proxy-app=persistent_kvstore` @@ -134,10 +134,10 @@ tendermint show_node_id --home ./mytestnet/node3 Finally, from each machine, run: ```sh -tendermint node --home ./mytestnet/node0 --proxy-app=kvstore --p2p.persistent-peers="ID1@IP1:26656,ID2@IP2:26656,ID3@IP3:26656,ID4@IP4:26656" -tendermint node --home ./mytestnet/node1 --proxy-app=kvstore --p2p.persistent-peers="ID1@IP1:26656,ID2@IP2:26656,ID3@IP3:26656,ID4@IP4:26656" -tendermint node --home ./mytestnet/node2 --proxy-app=kvstore --p2p.persistent-peers="ID1@IP1:26656,ID2@IP2:26656,ID3@IP3:26656,ID4@IP4:26656" -tendermint node --home ./mytestnet/node3 --proxy-app=kvstore --p2p.persistent-peers="ID1@IP1:26656,ID2@IP2:26656,ID3@IP3:26656,ID4@IP4:26656" +tendermint start --home ./mytestnet/node0 --proxy-app=kvstore --p2p.persistent-peers="ID1@IP1:26656,ID2@IP2:26656,ID3@IP3:26656,ID4@IP4:26656" +tendermint start --home ./mytestnet/node1 --proxy-app=kvstore --p2p.persistent-peers="ID1@IP1:26656,ID2@IP2:26656,ID3@IP3:26656,ID4@IP4:26656" +tendermint start --home ./mytestnet/node2 --proxy-app=kvstore --p2p.persistent-peers="ID1@IP1:26656,ID2@IP2:26656,ID3@IP3:26656,ID4@IP4:26656" +tendermint start --home ./mytestnet/node3 --proxy-app=kvstore --p2p.persistent-peers="ID1@IP1:26656,ID2@IP2:26656,ID3@IP3:26656,ID4@IP4:26656" ``` Note that after the third node is started, blocks will start to stream in diff --git a/docs/tendermint-core/using-tendermint.md b/docs/tendermint-core/using-tendermint.md index 749c0dc4b..f743a5dec 100644 --- a/docs/tendermint-core/using-tendermint.md +++ b/docs/tendermint-core/using-tendermint.md @@ -127,7 +127,7 @@ definition](https://github.com/tendermint/tendermint/blob/master/types/genesis.g To run a Tendermint node, use: ```bash -tendermint node +tendermint start ``` By default, Tendermint will try to connect to an ABCI application on @@ -136,7 +136,7 @@ another window. If you don't, kill Tendermint and run an in-process version of the `kvstore` app: ```bash -tendermint node --proxy-app=kvstore +tendermint start --proxy-app=kvstore ``` After a few seconds, you should see blocks start streaming in. Note that blocks @@ -150,10 +150,10 @@ Go, run it in another process, and use the `--proxy-app` flag to specify the address of the socket it is listening on, for instance: ```bash -tendermint node --proxy-app=/var/run/abci.sock +tendermint start --proxy-app=/var/run/abci.sock ``` -You can find out what flags are supported by running `tendermint node --help`. +You can find out what flags are supported by running `tendermint start --help`. ## Transactions @@ -270,7 +270,7 @@ transactions or the app hash changes, run Tendermint with this additional flag: ```sh -tendermint node --consensus.create_empty_blocks=false +tendermint start --consensus.create_empty_blocks=false ``` or set the configuration via the `config.toml` file: @@ -445,7 +445,7 @@ persistent connections with. For example, ```sh -tendermint node --p2p.seeds "f9baeaa15fedf5e1ef7448dd60f46c01f1a9e9c4@1.2.3.4:26656,0491d373a8e0fcf1023aaf18c51d6a1d0d4f31bd@5.6.7.8:26656" +tendermint start --p2p.seeds "f9baeaa15fedf5e1ef7448dd60f46c01f1a9e9c4@1.2.3.4:26656,0491d373a8e0fcf1023aaf18c51d6a1d0d4f31bd@5.6.7.8:26656" ``` Alternatively, you can use the `/dial_seeds` endpoint of the RPC to @@ -465,7 +465,7 @@ maintain a persistent connection with each, you can use the stopping Tendermint core instance. ```sh -tendermint node --p2p.persistent-peers "429fcf25974313b95673f58d77eacdd434402665@10.11.12.13:26656,96663a3dd0d7b9d17d4c8211b191af259621c693@10.11.12.14:26656" +tendermint start --p2p.persistent-peers "429fcf25974313b95673f58d77eacdd434402665@10.11.12.13:26656,96663a3dd0d7b9d17d4c8211b191af259621c693@10.11.12.14:26656" curl 'localhost:26657/dial_peers?persistent=true&peers=\["429fcf25974313b95673f58d77eacdd434402665@10.11.12.13:26656","96663a3dd0d7b9d17d4c8211b191af259621c693@10.11.12.14:26656"\]' ``` @@ -543,7 +543,7 @@ Update the `genesis.json` in `~/.tendermint/config`. Copy the genesis file and the new `priv_validator_key.json` to the `~/.tendermint/config` on a new machine. -Now run `tendermint node` on both machines, and use either +Now run `tendermint start` on both machines, and use either `--p2p.persistent-peers` or the `/dial_peers` to get them to peer up. They should start making blocks, and will only continue to do so as long as both of them are online. From 9ecfcc93a6364bbecc9e1d7740b53602eda667eb Mon Sep 17 00:00:00 2001 From: Frojdi Dymylja <33157909+fdymylja@users.noreply.github.com> Date: Mon, 29 Mar 2021 11:24:52 +0200 Subject: [PATCH 05/24] fix: jsonrpc url parsing and dial function (#6264) This PR fixes how the jsonrpc parses the URL, and how the dial function connects to the RPC. Closes: https://github.com/tendermint/tendermint/issues/6260 --- rpc/jsonrpc/client/http_json_client.go | 34 ++++++++++++-- rpc/jsonrpc/client/http_json_client_test.go | 50 +++++++++++++++++++++ 2 files changed, 81 insertions(+), 3 deletions(-) diff --git a/rpc/jsonrpc/client/http_json_client.go b/rpc/jsonrpc/client/http_json_client.go index 7f6a9a74a..295cdc24f 100644 --- a/rpc/jsonrpc/client/http_json_client.go +++ b/rpc/jsonrpc/client/http_json_client.go @@ -21,6 +21,7 @@ const ( protoWSS = "wss" protoWS = "ws" protoTCP = "tcp" + protoUNIX = "unix" ) //------------------------------------------------------------- @@ -28,6 +29,8 @@ const ( // Parsed URL structure type parsedURL struct { url.URL + + isUnixSocket bool } // Parse URL and set defaults @@ -42,7 +45,16 @@ func newParsedURL(remoteAddr string) (*parsedURL, error) { u.Scheme = protoTCP } - return &parsedURL{*u}, nil + pu := &parsedURL{ + URL: *u, + isUnixSocket: false, + } + + if u.Scheme == protoUNIX { + pu.isUnixSocket = true + } + + return pu, nil } // Change protocol to HTTP for unknown protocols and TCP protocol - useful for RPC connections @@ -65,10 +77,26 @@ func (u parsedURL) GetHostWithPath() string { // Get a trimmed address - useful for WS connections func (u parsedURL) GetTrimmedHostWithPath() string { - // replace / with . for http requests (kvstore domain) + // if it's not an unix socket we return the normal URL + if !u.isUnixSocket { + return u.GetHostWithPath() + } + // if it's a unix socket we replace the host slashes with a period + // this is because otherwise the http.Client would think that the + // domain is invalid. return strings.ReplaceAll(u.GetHostWithPath(), "/", ".") } +// GetDialAddress returns the endpoint to dial for the parsed URL +func (u parsedURL) GetDialAddress() string { + // if it's not a unix socket we return the host, example: localhost:443 + if !u.isUnixSocket { + return u.Host + } + // otherwise we return the path of the unix socket, ex /tmp/socket + return u.GetHostWithPath() +} + // Get a trimmed address with protocol - useful as address in RPC connections func (u parsedURL) GetTrimmedURL() string { return u.Scheme + "://" + u.GetTrimmedHostWithPath() @@ -350,7 +378,7 @@ func makeHTTPDialer(remoteAddr string) (func(string, string) (net.Conn, error), } dialFn := func(proto, addr string) (net.Conn, error) { - return net.Dial(protocol, u.GetHostWithPath()) + return net.Dial(protocol, u.GetDialAddress()) } return dialFn, nil diff --git a/rpc/jsonrpc/client/http_json_client_test.go b/rpc/jsonrpc/client/http_json_client_test.go index 5c8ef1a25..4b82ff1eb 100644 --- a/rpc/jsonrpc/client/http_json_client_test.go +++ b/rpc/jsonrpc/client/http_json_client_test.go @@ -34,3 +34,53 @@ func TestHTTPClientMakeHTTPDialer(t *testing.T) { require.NotNil(t, addr) } } + +func Test_parsedURL(t *testing.T) { + type test struct { + url string + expectedURL string + expectedHostWithPath string + expectedDialAddress string + } + + tests := map[string]test{ + "unix endpoint": { + url: "unix:///tmp/test", + expectedURL: "unix://.tmp.test", + expectedHostWithPath: "/tmp/test", + expectedDialAddress: "/tmp/test", + }, + + "http endpoint": { + url: "https://example.com", + expectedURL: "https://example.com", + expectedHostWithPath: "example.com", + expectedDialAddress: "example.com", + }, + + "http endpoint with port": { + url: "https://example.com:8080", + expectedURL: "https://example.com:8080", + expectedHostWithPath: "example.com:8080", + expectedDialAddress: "example.com:8080", + }, + + "http path routed endpoint": { + url: "https://example.com:8080/rpc", + expectedURL: "https://example.com:8080/rpc", + expectedHostWithPath: "example.com:8080/rpc", + expectedDialAddress: "example.com:8080", + }, + } + + for name, tt := range tests { + tt := tt // suppressing linter + t.Run(name, func(t *testing.T) { + parsed, err := newParsedURL(tt.url) + require.NoError(t, err) + require.Equal(t, tt.expectedDialAddress, parsed.GetDialAddress()) + require.Equal(t, tt.expectedURL, parsed.GetTrimmedURL()) + require.Equal(t, tt.expectedHostWithPath, parsed.GetHostWithPath()) + }) + } +} From 6a34fd8fc3e05f9540321e592ea8ff5f008c8321 Mon Sep 17 00:00:00 2001 From: Marko Date: Mon, 29 Mar 2021 13:38:26 +0000 Subject: [PATCH 06/24] ci: add janitor (#6292) * add janitor * add workflow ids * add comment --- .github/workflows/janitor.yml | 16 ++++++++++++++++ .github/workflows/tests.yml | 8 -------- 2 files changed, 16 insertions(+), 8 deletions(-) create mode 100644 .github/workflows/janitor.yml diff --git a/.github/workflows/janitor.yml b/.github/workflows/janitor.yml new file mode 100644 index 000000000..172ac07dd --- /dev/null +++ b/.github/workflows/janitor.yml @@ -0,0 +1,16 @@ +name: Janitor +# Janitor cleans up previous runs of various workflows +# To add more workflows to cancel visit https://api.github.com/repos/tendermint/tendermint/actions/workflows and find the actions name +on: + pull_request: + +jobs: + cancel: + name: "Cancel Previous Runs" + runs-on: ubuntu-latest + timeout-minutes: 3 + steps: + - uses: styfle/cancel-workflow-action@0.8.0 + with: + workflow_id: 1041851,1401230,2837803 + access_token: ${{ github.token }} diff --git a/.github/workflows/tests.yml b/.github/workflows/tests.yml index febdf33e9..5d978a3e2 100644 --- a/.github/workflows/tests.yml +++ b/.github/workflows/tests.yml @@ -10,14 +10,6 @@ on: - release/** jobs: - cleanup-runs: - runs-on: ubuntu-latest - steps: - - uses: rokroskar/workflow-run-cleanup-action@master - env: - GITHUB_TOKEN: "${{ secrets.GITHUB_TOKEN }}" - if: "!startsWith(github.ref, 'refs/tags/') && github.ref != 'refs/heads/master'" - build: name: Build runs-on: ubuntu-latest From 32ee737d42d3092dec6c99729927483abc1bd959 Mon Sep 17 00:00:00 2001 From: Marko Date: Mon, 29 Mar 2021 13:50:12 +0000 Subject: [PATCH 07/24] change index block log to info (#6290) ## Description Change log from error to info for indexing blocks --- state/txindex/indexer_service.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/state/txindex/indexer_service.go b/state/txindex/indexer_service.go index e72601252..9f2541b8b 100644 --- a/state/txindex/indexer_service.go +++ b/state/txindex/indexer_service.go @@ -79,7 +79,7 @@ func (is *IndexerService) OnStart() error { if err := is.blockIdxr.Index(eventDataHeader); err != nil { is.Logger.Error("failed to index block", "height", height, "err", err) } else { - is.Logger.Error("indexed block", "height", height) + is.Logger.Info("indexed block", "height", height) } if err = is.txIdxr.AddBatch(batch); err != nil { From cbae3613dd19198d609b282c15134e50f25c0d9f Mon Sep 17 00:00:00 2001 From: Callum Waters Date: Mon, 29 Mar 2021 20:00:20 +0200 Subject: [PATCH 08/24] e2e: add evidence generation and testing (#6276) --- .gitignore | 1 - CONTRIBUTING.md | 9 - test/e2e/Makefile | 7 +- test/e2e/app/main.go | 41 ----- test/e2e/docker/Dockerfile | 2 - test/e2e/docker/entrypoint-maverick | 10 - test/e2e/generator/generate.go | 36 +--- test/e2e/generator/random.go | 22 --- test/e2e/networks/ci.toml | 14 +- test/e2e/pkg/manifest.go | 18 +- test/e2e/pkg/testnet.go | 57 +----- test/e2e/runner/evidence.go | 274 ++++++++++++++++++++++++++++ test/e2e/runner/main.go | 35 +++- test/e2e/runner/setup.go | 35 +--- test/e2e/tests/evidence_test.go | 51 +----- 15 files changed, 337 insertions(+), 275 deletions(-) delete mode 100755 test/e2e/docker/entrypoint-maverick create mode 100644 test/e2e/runner/evidence.go diff --git a/.gitignore b/.gitignore index 40bfa28d8..b2a29d086 100644 --- a/.gitignore +++ b/.gitignore @@ -38,7 +38,6 @@ terraform.tfstate.d test/e2e/build test/e2e/networks/*/ test/logs -test/maverick/maverick test/p2p/data/ vendor test/fuzz/**/corpus diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index acaa44867..c3cc05330 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -366,15 +366,6 @@ cd test/e2e && \ ./build/runner -f networks/ci.toml ``` -### Maverick - -**If you're changing the code in `consensus` package, please make sure to -replicate all the changes in `./test/maverick/consensus`**. Maverick is a -byzantine node used to assert that the validator gets punished for malicious -behavior. - -See [README](./test/maverick/README.md) for details. - ### Model-based tests (ADVANCED) *NOTE: if you're just submitting your first PR, you won't need to touch these diff --git a/test/e2e/Makefile b/test/e2e/Makefile index c9eb8bc19..38ce809e6 100644 --- a/test/e2e/Makefile +++ b/test/e2e/Makefile @@ -8,11 +8,6 @@ docker: # ABCI testing). app: go build -o build/app -tags badgerdb,boltdb,cleveldb,rocksdb ./app - -# To be used primarily by the e2e docker instance. If you want to produce this binary -# elsewhere, then run go build in the maverick directory. -maverick: - go build -o build/maverick -tags badgerdb,boltdb,cleveldb,rocksdb ../maverick generator: go build -o build/generator ./generator @@ -20,4 +15,4 @@ generator: runner: go build -o build/runner ./runner -.PHONY: all app docker generator maverick runner +.PHONY: all app docker generator runner diff --git a/test/e2e/app/main.go b/test/e2e/app/main.go index 5141750ed..cb49b8303 100644 --- a/test/e2e/app/main.go +++ b/test/e2e/app/main.go @@ -83,10 +83,6 @@ func run(configFile string) error { default: err = startNode(cfg) } - // FIXME: Temporarily remove maverick until it is redesigned - // if len(cfg.Misbehaviors) == 0 { - // err = startMaverick(cfg) - // } default: err = fmt.Errorf("invalid protocol %q", cfg.Protocol) } @@ -227,43 +223,6 @@ func startLightNode(cfg *Config) error { return nil } -// FIXME: Temporarily disconnected maverick until it is redesigned -// startMaverick starts a Maverick node that runs the application directly. It assumes the Tendermint -// configuration is in $TMHOME/config/tendermint.toml. -// func startMaverick(cfg *Config) error { -// app, err := NewApplication(cfg) -// if err != nil { -// return err -// } - -// tmcfg, logger, nodeKey, err := setupNode() -// if err != nil { -// return fmt.Errorf("failed to setup config: %w", err) -// } - -// misbehaviors := make(map[int64]mcs.Misbehavior, len(cfg.Misbehaviors)) -// for heightString, misbehaviorString := range cfg.Misbehaviors { -// height, _ := strconv.ParseInt(heightString, 10, 64) -// misbehaviors[height] = mcs.MisbehaviorList[misbehaviorString] -// } - -// n, err := maverick.NewNode(tmcfg, -// maverick.LoadOrGenFilePV(tmcfg.PrivValidatorKeyFile(), tmcfg.PrivValidatorStateFile()), -// *nodeKey, -// proxy.NewLocalClientCreator(app), -// maverick.DefaultGenesisDocProviderFunc(tmcfg), -// maverick.DefaultDBProvider, -// maverick.DefaultMetricsProvider(tmcfg.Instrumentation), -// logger, -// misbehaviors, -// ) -// if err != nil { -// return err -// } - -// return n.Start() -// } - // startSigner starts a signer server connecting to the given endpoint. func startSigner(cfg *Config) error { filePV, err := privval.LoadFilePV(cfg.PrivValKey, cfg.PrivValState) diff --git a/test/e2e/docker/Dockerfile b/test/e2e/docker/Dockerfile index d0f9cd7a2..8c76f1d5e 100644 --- a/test/e2e/docker/Dockerfile +++ b/test/e2e/docker/Dockerfile @@ -19,8 +19,6 @@ COPY . . RUN make build && cp build/tendermint /usr/bin/tendermint COPY test/e2e/docker/entrypoint* /usr/bin/ -# FIXME: Temporarily disconnect maverick node until it is redesigned -# RUN cd test/e2e && make maverick && cp build/maverick /usr/bin/maverick RUN cd test/e2e && make app && cp build/app /usr/bin/app # Set up runtime directory. We don't use a separate runtime image since we need diff --git a/test/e2e/docker/entrypoint-maverick b/test/e2e/docker/entrypoint-maverick deleted file mode 100755 index 9469e2447..000000000 --- a/test/e2e/docker/entrypoint-maverick +++ /dev/null @@ -1,10 +0,0 @@ -#!/usr/bin/env bash - -# Forcibly remove any stray UNIX sockets left behind from previous runs -rm -rf /var/run/privval.sock /var/run/app.sock - -/usr/bin/app /tendermint/config/app.toml & - -sleep 1 - -/usr/bin/maverick "$@" diff --git a/test/e2e/generator/generate.go b/test/e2e/generator/generate.go index 29614fa3c..f71e71856 100644 --- a/test/e2e/generator/generate.go +++ b/test/e2e/generator/generate.go @@ -4,7 +4,6 @@ import ( "fmt" "math/rand" "sort" - "strconv" "strings" e2e "github.com/tendermint/tendermint/test/e2e/pkg" @@ -36,20 +35,14 @@ var ( nodeStateSyncs = uniformChoice{false, true} nodePersistIntervals = uniformChoice{0, 1, 5} nodeSnapshotIntervals = uniformChoice{0, 3} - nodeRetainBlocks = uniformChoice{0, 1, 5} + nodeRetainBlocks = uniformChoice{0, int(e2e.EvidenceAgeHeight), int(e2e.EvidenceAgeHeight) + 5} nodePerturbations = probSetChoice{ "disconnect": 0.1, "pause": 0.1, "kill": 0.1, "restart": 0.1, } - nodeMisbehaviors = weightedChoice{ - // FIXME: evidence disabled due to node panicing when not - // having sufficient block history to process evidence. - // https://github.com/tendermint/tendermint/issues/5617 - // misbehaviorOption{"double-prevote"}: 1, - misbehaviorOption{}: 9, - } + evidence = uniformChoice{0, 1, 10} ) // Generate generates random testnets using the given RNG. @@ -75,6 +68,7 @@ func generateTestnet(r *rand.Rand, opt map[string]interface{}) (e2e.Manifest, er ValidatorUpdates: map[string]map[string]int64{}, Nodes: map[string]*e2e.ManifestNode{}, KeyType: opt["keyType"].(string), + Evidence: evidence.Choose(r).(int), } var numSeeds, numValidators, numFulls, numLightClients int @@ -227,17 +221,6 @@ func generateNode( node.SnapshotInterval = 3 } - if node.Mode == string(e2e.ModeValidator) { - misbehaveAt := startAt + 5 + int64(r.Intn(10)) - if startAt == 0 { - misbehaveAt += initialHeight - 1 - } - node.Misbehaviors = nodeMisbehaviors.Choose(r).(misbehaviorOption).atHeight(misbehaveAt) - if len(node.Misbehaviors) != 0 { - node.PrivvalProtocol = "file" - } - } - // If a node which does not persist state also does not retain blocks, randomly // choose to either persist state or retain all blocks. if node.PersistInterval != nil && *node.PersistInterval == 0 && node.RetainBlocks > 0 { @@ -276,16 +259,3 @@ func generateLightNode(r *rand.Rand, startAt int64, providers []string) *e2e.Man func ptrUint64(i uint64) *uint64 { return &i } - -type misbehaviorOption struct { - misbehavior string -} - -func (m misbehaviorOption) atHeight(height int64) map[string]string { - misbehaviorMap := make(map[string]string) - if m.misbehavior == "" { - return misbehaviorMap - } - misbehaviorMap[strconv.Itoa(int(height))] = m.misbehavior - return misbehaviorMap -} diff --git a/test/e2e/generator/random.go b/test/e2e/generator/random.go index ec59a01b2..f21502118 100644 --- a/test/e2e/generator/random.go +++ b/test/e2e/generator/random.go @@ -56,28 +56,6 @@ func (uc uniformChoice) Choose(r *rand.Rand) interface{} { return uc[r.Intn(len(uc))] } -// weightedChoice chooses a single random key from a map of keys and weights. -type weightedChoice map[interface{}]uint - -func (wc weightedChoice) Choose(r *rand.Rand) interface{} { - total := 0 - choices := make([]interface{}, 0, len(wc)) - for choice, weight := range wc { - total += int(weight) - choices = append(choices, choice) - } - - rem := r.Intn(total) - for _, choice := range choices { - rem -= int(wc[choice]) - if rem <= 0 { - return choice - } - } - - return nil -} - // probSetChoice picks a set of strings based on each string's probability (0-1). type probSetChoice map[string]float64 diff --git a/test/e2e/networks/ci.toml b/test/e2e/networks/ci.toml index 879d0c22f..05875dace 100644 --- a/test/e2e/networks/ci.toml +++ b/test/e2e/networks/ci.toml @@ -2,6 +2,7 @@ # functionality with a single network. initial_height = 1000 +evidence = 0 initial_state = { initial01 = "a", initial02 = "b", initial03 = "c" } [validators] @@ -37,8 +38,6 @@ seeds = ["seed01"] seeds = ["seed01"] snapshot_interval = 5 perturb = ["disconnect"] -# FIXME: maverick has been disabled until it is redesigned (https://github.com/tendermint/tendermint/issues/5575) -# misbehaviors = { 1018 = "double-prevote" } [node.validator02] seeds = ["seed02"] @@ -80,7 +79,7 @@ mode = "full" # FIXME: should be v2, disabled due to flake fast_sync = "v0" persistent_peers = ["validator01", "validator02", "validator03", "validator04", "validator05"] -retain_blocks = 1 +retain_blocks = 3 perturb = ["restart"] [node.full02] @@ -94,10 +93,5 @@ perturb = ["restart"] [node.light01] mode= "light" -start_at= 1005 -persistent_peers = ["validator01", "validator02", "validator03"] - -[node.light02] -mode= "light" -start_at= 1015 -persistent_peers = ["validator04", "full01", "validator05"] \ No newline at end of file +start_at= 1010 +persistent_peers = ["validator01", "validator02", "validator03"] \ No newline at end of file diff --git a/test/e2e/pkg/manifest.go b/test/e2e/pkg/manifest.go index 8fbaea185..84628594f 100644 --- a/test/e2e/pkg/manifest.go +++ b/test/e2e/pkg/manifest.go @@ -51,6 +51,10 @@ type Manifest struct { // Options are ed25519 & secp256k1 KeyType string `toml:"key_type"` + // Evidence indicates the amount of evidence that will be injected into the + // testnet via the RPC endpoint of a random node. Default is 0 + Evidence int `toml:"evidence"` + // LogLevel sets the log level of the entire testnet. This can be overridden // by individual nodes. LogLevel string `toml:"log_level"` @@ -113,8 +117,8 @@ type ManifestNode struct { SnapshotInterval uint64 `toml:"snapshot_interval"` // RetainBlocks specifies the number of recent blocks to retain. Defaults to - // 0, which retains all blocks. Must be greater that PersistInterval and - // SnapshotInterval. + // 0, which retains all blocks. Must be greater that PersistInterval, + // SnapshotInterval and EvidenceAgeHeight. RetainBlocks uint64 `toml:"retain_blocks"` // Perturb lists perturbations to apply to the node after it has been @@ -126,16 +130,6 @@ type ManifestNode struct { // restart: restarts the node, shutting it down with SIGTERM Perturb []string `toml:"perturb"` - // Misbehaviors sets how a validator behaves during consensus at a - // certain height. Multiple misbehaviors at different heights can be used - // - // An example of misbehaviors - // { 10 = "double-prevote", 20 = "double-prevote"} - // - // For more information, look at the readme in the maverick folder. - // A list of all behaviors can be found in ../maverick/consensus/behavior.go - Misbehaviors map[string]string `toml:"misbehaviors"` - // Log level sets the log level of the specific node i.e. "consensus:info,*:error". // This is helpful when debugging a specific problem. This overrides the network // level. diff --git a/test/e2e/pkg/testnet.go b/test/e2e/pkg/testnet.go index b773a158a..1e3eede90 100644 --- a/test/e2e/pkg/testnet.go +++ b/test/e2e/pkg/testnet.go @@ -11,6 +11,7 @@ import ( "sort" "strconv" "strings" + "time" "github.com/tendermint/tendermint/crypto" "github.com/tendermint/tendermint/crypto/ed25519" @@ -46,6 +47,9 @@ const ( PerturbationKill Perturbation = "kill" PerturbationPause Perturbation = "pause" PerturbationRestart Perturbation = "restart" + + EvidenceAgeHeight int64 = 3 + EvidenceAgeTime time.Duration = 10 * time.Second ) // Testnet represents a single testnet. @@ -60,6 +64,7 @@ type Testnet struct { ValidatorUpdates map[int64]map[*Node]int64 Nodes []*Node KeyType string + Evidence int LogLevel string } @@ -84,7 +89,6 @@ type Node struct { Seeds []*Node PersistentPeers []*Node Perturbations []Perturbation - Misbehaviors map[int64]string LogLevel string } @@ -124,6 +128,7 @@ func LoadTestnet(file string) (*Testnet, error) { Validators: map[*Node]int64{}, ValidatorUpdates: map[int64]map[*Node]int64{}, Nodes: []*Node{}, + Evidence: manifest.Evidence, KeyType: "ed25519", LogLevel: manifest.LogLevel, } @@ -161,7 +166,6 @@ func LoadTestnet(file string) (*Testnet, error) { SnapshotInterval: nodeManifest.SnapshotInterval, RetainBlocks: nodeManifest.RetainBlocks, Perturbations: []Perturbation{}, - Misbehaviors: make(map[int64]string), LogLevel: manifest.LogLevel, } if node.StartAt == testnet.InitialHeight { @@ -185,13 +189,6 @@ func LoadTestnet(file string) (*Testnet, error) { for _, p := range nodeManifest.Perturb { node.Perturbations = append(node.Perturbations, Perturbation(p)) } - for heightString, misbehavior := range nodeManifest.Misbehaviors { - height, err := strconv.ParseInt(heightString, 10, 64) - if err != nil { - return nil, fmt.Errorf("unable to parse height %s to int64: %w", heightString, err) - } - node.Misbehaviors[height] = misbehavior - } if nodeManifest.LogLevel != "" { node.LogLevel = nodeManifest.LogLevel } @@ -344,6 +341,10 @@ func (n Node) Validate(testnet Testnet) error { if n.StateSync && n.StartAt == 0 { return errors.New("state synced nodes cannot start at the initial height") } + if n.RetainBlocks != 0 && n.RetainBlocks < uint64(EvidenceAgeHeight) { + return fmt.Errorf("retain_blocks must be greater or equal to max evidence age (%d)", + EvidenceAgeHeight) + } if n.PersistInterval == 0 && n.RetainBlocks > 0 { return errors.New("persist_interval=0 requires retain_blocks=0") } @@ -362,31 +363,6 @@ func (n Node) Validate(testnet Testnet) error { } } - if (n.PrivvalProtocol != "file" || n.Mode != "validator") && len(n.Misbehaviors) != 0 { - return errors.New("must be using \"file\" privval protocol to implement misbehaviors") - } - - for height, misbehavior := range n.Misbehaviors { - if height < n.StartAt { - return fmt.Errorf("misbehavior height %d is below node start height %d", - height, n.StartAt) - } - if height < testnet.InitialHeight { - return fmt.Errorf("misbehavior height %d is below network initial height %d", - height, testnet.InitialHeight) - } - exists := false - // FIXME: Maverick has been disabled until it is redesigned - // for possibleBehaviors := range mcs.MisbehaviorList { - // if possibleBehaviors == misbehavior { - // exists = true - // } - // } - if !exists { - return fmt.Errorf("misbehavior %s does not exist", misbehavior) - } - } - return nil } @@ -438,19 +414,6 @@ func (t Testnet) HasPerturbations() bool { return false } -// LastMisbehaviorHeight returns the height of the last misbehavior. -func (t Testnet) LastMisbehaviorHeight() int64 { - lastHeight := int64(0) - for _, node := range t.Nodes { - for height := range node.Misbehaviors { - if height > lastHeight { - lastHeight = height - } - } - } - return lastHeight -} - // Address returns a P2P endpoint address for the node. func (n Node) AddressP2P(withID bool) string { ip := n.IP.String() diff --git a/test/e2e/runner/evidence.go b/test/e2e/runner/evidence.go new file mode 100644 index 000000000..ece241cd2 --- /dev/null +++ b/test/e2e/runner/evidence.go @@ -0,0 +1,274 @@ +package main + +import ( + "bytes" + "context" + "fmt" + "io/ioutil" + "math/rand" + "path/filepath" + "time" + + "github.com/tendermint/tendermint/crypto" + "github.com/tendermint/tendermint/crypto/tmhash" + tmjson "github.com/tendermint/tendermint/libs/json" + "github.com/tendermint/tendermint/privval" + tmproto "github.com/tendermint/tendermint/proto/tendermint/types" + e2e "github.com/tendermint/tendermint/test/e2e/pkg" + "github.com/tendermint/tendermint/types" + "github.com/tendermint/tendermint/version" +) + +// 1 in 11 evidence is light client evidence, the rest is duplicate vote +// FIXME: Setting to 11 disables light client attack evidence since nodes +// don't follow a minimum retention height invariant. When we fix this we +// should use a ratio of 4. +const lightClientEvidenceRatio = 11 + +// InjectEvidence takes a running testnet and generates an amount of valid +// evidence and broadcasts it to a random node through the rpc endpoint `/broadcast_evidence`. +// Evidence is random and can be a mixture of LightClientAttackEvidence and +// DuplicateVoteEvidence. +func InjectEvidence(testnet *e2e.Testnet, amount int) error { + // select a random node + targetNode := testnet.RandomNode() + + logger.Info(fmt.Sprintf("Injecting evidence through %v (amount: %d)...", targetNode.Name, amount)) + + client, err := targetNode.Client() + if err != nil { + return err + } + + // request the latest block and validator set from the node + blockRes, err := client.Block(context.Background(), nil) + if err != nil { + return err + } + lightEvidenceCommonHeight := blockRes.Block.Height + waitHeight := blockRes.Block.Height + 3 + duplicateVoteHeight := waitHeight + + nValidators := 100 + valRes, err := client.Validators(context.Background(), &lightEvidenceCommonHeight, nil, &nValidators) + if err != nil { + return err + } + valSet, err := types.ValidatorSetFromExistingValidators(valRes.Validators) + if err != nil { + return err + } + + // get the private keys of all the validators in the network + privVals, err := getPrivateValidatorKeys(testnet) + if err != nil { + return err + } + + // wait for the node to reach the height above the forged height so that + // it is able to validate the evidence + status, err := waitForNode(targetNode, waitHeight, 10*time.Second) + if err != nil { + return err + } + duplicateVoteTime := status.SyncInfo.LatestBlockTime + + var ev types.Evidence + for i := 0; i < amount; i++ { + if i%lightClientEvidenceRatio == 0 { + ev, err = generateLightClientAttackEvidence( + privVals, lightEvidenceCommonHeight, valSet, testnet.Name, blockRes.Block.Time, + ) + } else { + ev, err = generateDuplicateVoteEvidence( + privVals, duplicateVoteHeight, valSet, testnet.Name, duplicateVoteTime, + ) + } + if err != nil { + return err + } + + _, err := client.BroadcastEvidence(context.Background(), ev) + if err != nil { + return err + } + } + + logger.Info(fmt.Sprintf("Finished sending evidence (height %d)", blockRes.Block.Height+2)) + + return nil +} + +func getPrivateValidatorKeys(testnet *e2e.Testnet) ([]types.MockPV, error) { + privVals := []types.MockPV{} + + for _, node := range testnet.Nodes { + if node.Mode == e2e.ModeValidator { + privKeyPath := filepath.Join(testnet.Dir, node.Name, PrivvalKeyFile) + privKey, err := readPrivKey(privKeyPath) + if err != nil { + return nil, err + } + // Create mock private validators from the validators private key. MockPV is + // stateless which means we can double vote and do other funky stuff + privVals = append(privVals, types.NewMockPVWithParams(privKey, false, false)) + } + } + + return privVals, nil +} + +// creates evidence of a lunatic attack. The height provided is the common height. +// The forged height happens 2 blocks later. +func generateLightClientAttackEvidence( + privVals []types.MockPV, + height int64, + vals *types.ValidatorSet, + chainID string, + evTime time.Time, +) (*types.LightClientAttackEvidence, error) { + // forge a random header + forgedHeight := height + 2 + forgedTime := evTime.Add(1 * time.Second) + header := makeHeaderRandom(chainID, forgedHeight) + header.Time = forgedTime + + // add a new bogus validator and remove an existing one to + // vary the validator set slightly + pv, conflictingVals, err := mutateValidatorSet(privVals, vals) + if err != nil { + return nil, err + } + + header.ValidatorsHash = conflictingVals.Hash() + + // create a commit for the forged header + blockID := makeBlockID(header.Hash(), 1000, []byte("partshash")) + voteSet := types.NewVoteSet(chainID, forgedHeight, 0, tmproto.SignedMsgType(2), conflictingVals) + commit, err := types.MakeCommit(blockID, forgedHeight, 0, voteSet, pv, forgedTime) + if err != nil { + return nil, err + } + + ev := &types.LightClientAttackEvidence{ + ConflictingBlock: &types.LightBlock{ + SignedHeader: &types.SignedHeader{ + Header: header, + Commit: commit, + }, + ValidatorSet: conflictingVals, + }, + CommonHeight: height, + TotalVotingPower: vals.TotalVotingPower(), + Timestamp: evTime, + } + ev.ByzantineValidators = ev.GetByzantineValidators(vals, &types.SignedHeader{ + Header: makeHeaderRandom(chainID, forgedHeight), + }) + return ev, nil +} + +// generateDuplicateVoteEvidence picks a random validator from the val set and +// returns duplicate vote evidence against the validator +func generateDuplicateVoteEvidence( + privVals []types.MockPV, + height int64, + vals *types.ValidatorSet, + chainID string, + time time.Time, +) (*types.DuplicateVoteEvidence, error) { + // nolint:gosec // G404: Use of weak random number generator + privVal := privVals[rand.Intn(len(privVals))] + voteA, err := types.MakeVote(height, makeRandomBlockID(), vals, privVal, chainID, time) + if err != nil { + return nil, err + } + voteB, err := types.MakeVote(height, makeRandomBlockID(), vals, privVal, chainID, time) + if err != nil { + return nil, err + } + return types.NewDuplicateVoteEvidence(voteA, voteB, time, vals), nil +} + +func readPrivKey(keyFilePath string) (crypto.PrivKey, error) { + keyJSONBytes, err := ioutil.ReadFile(keyFilePath) + if err != nil { + return nil, err + } + pvKey := privval.FilePVKey{} + err = tmjson.Unmarshal(keyJSONBytes, &pvKey) + if err != nil { + return nil, fmt.Errorf("error reading PrivValidator key from %v: %w", keyFilePath, err) + } + + return pvKey.PrivKey, nil +} + +func makeHeaderRandom(chainID string, height int64) *types.Header { + return &types.Header{ + Version: version.Consensus{Block: version.BlockProtocol, App: 1}, + ChainID: chainID, + Height: height, + Time: time.Now(), + LastBlockID: makeBlockID([]byte("headerhash"), 1000, []byte("partshash")), + LastCommitHash: crypto.CRandBytes(tmhash.Size), + DataHash: crypto.CRandBytes(tmhash.Size), + ValidatorsHash: crypto.CRandBytes(tmhash.Size), + NextValidatorsHash: crypto.CRandBytes(tmhash.Size), + ConsensusHash: crypto.CRandBytes(tmhash.Size), + AppHash: crypto.CRandBytes(tmhash.Size), + LastResultsHash: crypto.CRandBytes(tmhash.Size), + EvidenceHash: crypto.CRandBytes(tmhash.Size), + ProposerAddress: crypto.CRandBytes(crypto.AddressSize), + } +} + +func makeRandomBlockID() types.BlockID { + return makeBlockID(crypto.CRandBytes(tmhash.Size), 100, crypto.CRandBytes(tmhash.Size)) +} + +func makeBlockID(hash []byte, partSetSize uint32, partSetHash []byte) types.BlockID { + var ( + h = make([]byte, tmhash.Size) + psH = make([]byte, tmhash.Size) + ) + copy(h, hash) + copy(psH, partSetHash) + return types.BlockID{ + Hash: h, + PartSetHeader: types.PartSetHeader{ + Total: partSetSize, + Hash: psH, + }, + } +} + +func mutateValidatorSet(privVals []types.MockPV, vals *types.ValidatorSet, +) ([]types.PrivValidator, *types.ValidatorSet, error) { + newVal, newPrivVal := types.RandValidator(false, 10) + + var newVals *types.ValidatorSet + if vals.Size() > 2 { + newVals = types.NewValidatorSet(append(vals.Copy().Validators[:vals.Size()-1], newVal)) + } else { + newVals = types.NewValidatorSet(append(vals.Copy().Validators, newVal)) + } + + // we need to sort the priv validators with the same index as the validator set + pv := make([]types.PrivValidator, newVals.Size()) + for idx, val := range newVals.Validators { + found := false + for _, p := range append(privVals, newPrivVal.(types.MockPV)) { + if bytes.Equal(p.PrivKey.PubKey().Address(), val.Address) { + pv[idx] = p + found = true + break + } + } + if !found { + return nil, nil, fmt.Errorf("missing priv validator for %v", val.Address) + } + } + + return pv, newVals, nil +} diff --git a/test/e2e/runner/main.go b/test/e2e/runner/main.go index 993df98ef..35b0d952f 100644 --- a/test/e2e/runner/main.go +++ b/test/e2e/runner/main.go @@ -71,14 +71,6 @@ func NewCLI() *CLI { return err } - if lastMisbehavior := cli.testnet.LastMisbehaviorHeight(); lastMisbehavior > 0 { - // wait for misbehaviors before starting perturbations. We do a separate - // wait for another 5 blocks, since the last misbehavior height may be - // in the past depending on network startup ordering. - if err := WaitUntil(cli.testnet, lastMisbehavior); err != nil { - return err - } - } if err := Wait(cli.testnet, 5); err != nil { // allow some txs to go through return err } @@ -92,6 +84,15 @@ func NewCLI() *CLI { } } + if cli.testnet.Evidence > 0 { + if err := InjectEvidence(cli.testnet, cli.testnet.Evidence); err != nil { + return err + } + if err := Wait(cli.testnet, 1); err != nil { // ensure chain progress + return err + } + } + loadCancel() if err := <-chLoadResult; err != nil { return err @@ -188,6 +189,24 @@ func NewCLI() *CLI { }, }) + cli.root.AddCommand(&cobra.Command{ + Use: "evidence [amount]", + Args: cobra.MaximumNArgs(1), + Short: "Generates and broadcasts evidence to a random node", + RunE: func(cmd *cobra.Command, args []string) (err error) { + amount := 1 + + if len(args) == 1 { + amount, err = strconv.Atoi(args[0]) + if err != nil { + return err + } + } + + return InjectEvidence(cli.testnet, amount) + }, + }) + cli.root.AddCommand(&cobra.Command{ Use: "test", Short: "Runs test cases against a running testnet", diff --git a/test/e2e/runner/setup.go b/test/e2e/runner/setup.go index 6045afbe4..22ff58b15 100644 --- a/test/e2e/runner/setup.go +++ b/test/e2e/runner/setup.go @@ -12,7 +12,6 @@ import ( "path/filepath" "regexp" "sort" - "strconv" "strings" "text/template" "time" @@ -132,28 +131,6 @@ func Setup(testnet *e2e.Testnet) error { func MakeDockerCompose(testnet *e2e.Testnet) ([]byte, error) { // Must use version 2 Docker Compose format, to support IPv6. tmpl, err := template.New("docker-compose").Funcs(template.FuncMap{ - "startCommands": func(misbehaviors map[int64]string, logLevel string) string { - command := "start" - - // FIXME: Temporarily disable behaviors until maverick is redesigned - // misbehaviorString := "" - // for height, misbehavior := range misbehaviors { - // // after the first behavior set, a comma must be prepended - // if misbehaviorString != "" { - // misbehaviorString += "," - // } - // heightString := strconv.Itoa(int(height)) - // misbehaviorString += misbehavior + "," + heightString - // } - - // if misbehaviorString != "" { - // command += " --misbehaviors " + misbehaviorString - // } - if logLevel != "" && logLevel != config.DefaultLogLevel { - command += " --log-level " + logLevel - } - return command - }, "addUint32": func(x, y uint32) uint32 { return x + y }, @@ -181,8 +158,8 @@ services: image: tendermint/e2e-node {{- if eq .ABCIProtocol "builtin" }} entrypoint: /usr/bin/entrypoint-builtin -{{- else }} - command: {{ startCommands .Misbehaviors .LogLevel }} +{{- else if .LogLevel }} + command: start --log-level {{ .LogLevel }} {{- end }} init: true ports: @@ -223,6 +200,8 @@ func MakeGenesis(testnet *e2e.Testnet) (types.GenesisDoc, error) { default: return genesis, errors.New("unsupported KeyType") } + genesis.ConsensusParams.Evidence.MaxAgeNumBlocks = e2e.EvidenceAgeHeight + genesis.ConsensusParams.Evidence.MaxAgeDuration = e2e.EvidenceAgeTime for validator, power := range testnet.Validators { genesis.Validators = append(genesis.Validators, types.GenesisValidator{ Name: validator.Name, @@ -403,12 +382,6 @@ func MakeAppConfig(node *e2e.Node) ([]byte, error) { } } - misbehaviors := make(map[string]string) - for height, misbehavior := range node.Misbehaviors { - misbehaviors[strconv.Itoa(int(height))] = misbehavior - } - cfg["misbehaviors"] = misbehaviors - if len(node.Testnet.ValidatorUpdates) > 0 { validatorUpdates := map[string]map[string]int64{} for height, validators := range node.Testnet.ValidatorUpdates { diff --git a/test/e2e/tests/evidence_test.go b/test/e2e/tests/evidence_test.go index ea24b51e5..f7f2ede79 100644 --- a/test/e2e/tests/evidence_test.go +++ b/test/e2e/tests/evidence_test.go @@ -1,57 +1,22 @@ package e2e_test import ( - "bytes" "testing" "github.com/stretchr/testify/require" - - e2e "github.com/tendermint/tendermint/test/e2e/pkg" - "github.com/tendermint/tendermint/types" ) // assert that all nodes that have blocks at the height of a misbehavior has evidence // for that misbehavior func TestEvidence_Misbehavior(t *testing.T) { blocks := fetchBlockChain(t) - testNode(t, func(t *testing.T, node e2e.Node) { - seenEvidence := make(map[int64]struct{}) - for _, block := range blocks { - // Find any evidence blaming this node in this block - var nodeEvidence types.Evidence - for _, evidence := range block.Evidence.Evidence { - switch evidence := evidence.(type) { - case *types.DuplicateVoteEvidence: - if bytes.Equal(evidence.VoteA.ValidatorAddress, node.PrivvalKey.PubKey().Address()) { - nodeEvidence = evidence - } - default: - t.Fatalf("unexpected evidence type %T", evidence) - } - } - if nodeEvidence == nil { - continue // no evidence for the node at this height - } - - // Check that evidence was as expected - misbehavior, ok := node.Misbehaviors[nodeEvidence.Height()] - require.True(t, ok, "found unexpected evidence %v in height %v", - nodeEvidence, block.Height) - - switch misbehavior { - case "double-prevote": - require.IsType(t, &types.DuplicateVoteEvidence{}, nodeEvidence, "unexpected evidence type") - default: - t.Fatalf("unknown misbehavior %v", misbehavior) - } - - seenEvidence[nodeEvidence.Height()] = struct{}{} + testnet := loadTestnet(t) + seenEvidence := 0 + for _, block := range blocks { + if len(block.Evidence.Evidence) != 0 { + seenEvidence += len(block.Evidence.Evidence) } - // see if there is any evidence that we were expecting but didn't see - for height, misbehavior := range node.Misbehaviors { - _, ok := seenEvidence[height] - require.True(t, ok, "expected evidence for %v misbehavior at height %v by node but was never found", - misbehavior, height) - } - }) + } + require.Equal(t, testnet.Evidence, seenEvidence, + "difference between the amount of evidence produced and committed") } From 91506bf25d385050f531982b20b98d2c09f8c2d2 Mon Sep 17 00:00:00 2001 From: Sam Kleinman Date: Mon, 29 Mar 2021 16:12:23 -0400 Subject: [PATCH 09/24] p2p: simple peer scoring (#6277) --- blockchain/v0/reactor_test.go | 2 +- consensus/reactor.go | 17 ++++---- evidence/reactor_test.go | 2 +- mempool/reactor_test.go | 2 +- p2p/peermanager.go | 78 +++++++++++++++++++++++++++++------ p2p/peermanager_test.go | 36 ++++++++++++++++ p2p/shim.go | 6 +-- statesync/reactor_test.go | 2 +- 8 files changed, 118 insertions(+), 27 deletions(-) diff --git a/blockchain/v0/reactor_test.go b/blockchain/v0/reactor_test.go index a92d6fad1..64a66a6a0 100644 --- a/blockchain/v0/reactor_test.go +++ b/blockchain/v0/reactor_test.go @@ -152,7 +152,7 @@ func (rts *reactorTestSuite) addNode(t *testing.T, } rts.peerChans[nodeID] = make(chan p2p.PeerUpdate) - rts.peerUpdates[nodeID] = p2p.NewPeerUpdates(rts.peerChans[nodeID]) + rts.peerUpdates[nodeID] = p2p.NewPeerUpdates(rts.peerChans[nodeID], 1) rts.network.Nodes[nodeID].PeerManager.Register(rts.peerUpdates[nodeID]) rts.reactors[nodeID], err = NewReactor( rts.logger.With("nodeID", nodeID), diff --git a/consensus/reactor.go b/consensus/reactor.go index 8185f4202..cf3f3e42d 100644 --- a/consensus/reactor.go +++ b/consensus/reactor.go @@ -1377,18 +1377,21 @@ func (r *Reactor) peerStatsRoutine() { switch msg.Msg.(type) { case *VoteMessage: - if numVotes := ps.RecordVote(); numVotes%votesToContributeToBecomeGoodPeer == 0 { // nolint: staticcheck - // TODO: Handle peer quality via the peer manager. - // r.Switch.MarkPeerAsGood(peer) + if numVotes := ps.RecordVote(); numVotes%votesToContributeToBecomeGoodPeer == 0 { + r.peerUpdates.SendUpdate(p2p.PeerUpdate{ + NodeID: msg.PeerID, + Status: p2p.PeerStatusGood, + }) } case *BlockPartMessage: - if numParts := ps.RecordBlockPart(); numParts%blocksToContributeToBecomeGoodPeer == 0 { // nolint: staticcheck - // TODO: Handle peer quality via the peer manager. - // r.Switch.MarkPeerAsGood(peer) + if numParts := ps.RecordBlockPart(); numParts%blocksToContributeToBecomeGoodPeer == 0 { + r.peerUpdates.SendUpdate(p2p.PeerUpdate{ + NodeID: msg.PeerID, + Status: p2p.PeerStatusGood, + }) } } - case <-r.closeCh: return } diff --git a/evidence/reactor_test.go b/evidence/reactor_test.go index 2170d7589..684a9f406 100644 --- a/evidence/reactor_test.go +++ b/evidence/reactor_test.go @@ -86,7 +86,7 @@ func setup(t *testing.T, stateStores []sm.Store, chBuf uint) *reactorTestSuite { require.NoError(t, err) rts.peerChans[nodeID] = make(chan p2p.PeerUpdate) - rts.peerUpdates[nodeID] = p2p.NewPeerUpdates(rts.peerChans[nodeID]) + rts.peerUpdates[nodeID] = p2p.NewPeerUpdates(rts.peerChans[nodeID], 1) rts.network.Nodes[nodeID].PeerManager.Register(rts.peerUpdates[nodeID]) rts.nodes = append(rts.nodes, rts.network.Nodes[nodeID]) diff --git a/mempool/reactor_test.go b/mempool/reactor_test.go index d1d41209e..e287302f0 100644 --- a/mempool/reactor_test.go +++ b/mempool/reactor_test.go @@ -60,7 +60,7 @@ func setup(t *testing.T, cfg *cfg.MempoolConfig, numNodes int, chBuf uint) *reac rts.mempools[nodeID] = mempool rts.peerChans[nodeID] = make(chan p2p.PeerUpdate) - rts.peerUpdates[nodeID] = p2p.NewPeerUpdates(rts.peerChans[nodeID]) + rts.peerUpdates[nodeID] = p2p.NewPeerUpdates(rts.peerChans[nodeID], 1) rts.network.Nodes[nodeID].PeerManager.Register(rts.peerUpdates[nodeID]) rts.reactors[nodeID] = NewReactor( diff --git a/p2p/peermanager.go b/p2p/peermanager.go index 3f80bf9d6..bbb1e31f9 100644 --- a/p2p/peermanager.go +++ b/p2p/peermanager.go @@ -33,13 +33,15 @@ type PeerStatus string const ( PeerStatusUp PeerStatus = "up" // connected and ready PeerStatusDown PeerStatus = "down" // disconnected + PeerStatusGood PeerStatus = "good" // peer observed as good + PeerStatusBad PeerStatus = "bad" // peer observed as bad ) // PeerScore is a numeric score assigned to a peer (higher is better). type PeerScore uint8 const ( - PeerScorePersistent PeerScore = 100 // persistent peers + PeerScorePersistent PeerScore = math.MaxUint8 // persistent peers ) // PeerUpdate is a peer update event sent via PeerUpdates. @@ -51,24 +53,35 @@ type PeerUpdate struct { // PeerUpdates is a peer update subscription with notifications about peer // events (currently just status changes). type PeerUpdates struct { - updatesCh chan PeerUpdate - closeCh chan struct{} - closeOnce sync.Once + routerUpdatesCh chan PeerUpdate + reactorUpdatesCh chan PeerUpdate + closeCh chan struct{} + closeOnce sync.Once } // NewPeerUpdates creates a new PeerUpdates subscription. It is primarily for // internal use, callers should typically use PeerManager.Subscribe(). The // subscriber must call Close() when done. -func NewPeerUpdates(updatesCh chan PeerUpdate) *PeerUpdates { +func NewPeerUpdates(updatesCh chan PeerUpdate, buf int) *PeerUpdates { return &PeerUpdates{ - updatesCh: updatesCh, - closeCh: make(chan struct{}), + reactorUpdatesCh: updatesCh, + routerUpdatesCh: make(chan PeerUpdate, buf), + closeCh: make(chan struct{}), } } // Updates returns a channel for consuming peer updates. func (pu *PeerUpdates) Updates() <-chan PeerUpdate { - return pu.updatesCh + return pu.reactorUpdatesCh +} + +// SendUpdate pushes information about a peer into the routing layer, +// presumably from a peer. +func (pu *PeerUpdates) SendUpdate(update PeerUpdate) { + select { + case <-pu.closeCh: + case pu.routerUpdatesCh <- update: + } } // Close closes the peer updates subscription. @@ -791,7 +804,7 @@ func (m *PeerManager) Subscribe() *PeerUpdates { // to the next subscriptions. This also prevents tail latencies from // compounding. Limiting it to 1 means that the subscribers are still // reasonably in sync. However, this should probably be benchmarked. - peerUpdates := NewPeerUpdates(make(chan PeerUpdate, 1)) + peerUpdates := NewPeerUpdates(make(chan PeerUpdate, 1), 1) m.Register(peerUpdates) return peerUpdates } @@ -809,6 +822,19 @@ func (m *PeerManager) Register(peerUpdates *PeerUpdates) { m.subscriptions[peerUpdates] = peerUpdates m.mtx.Unlock() + go func() { + for { + select { + case <-peerUpdates.closeCh: + return + case <-m.closeCh: + return + case pu := <-peerUpdates.routerUpdatesCh: + m.processPeerEvent(pu) + } + } + }() + go func() { select { case <-peerUpdates.Done(): @@ -820,6 +846,22 @@ func (m *PeerManager) Register(peerUpdates *PeerUpdates) { }() } +func (m *PeerManager) processPeerEvent(pu PeerUpdate) { + m.mtx.Lock() + defer m.mtx.Unlock() + + if _, ok := m.store.peers[pu.NodeID]; !ok { + m.store.peers[pu.NodeID] = &peerInfo{} + } + + switch pu.Status { + case PeerStatusBad: + m.store.peers[pu.NodeID].MutableScore-- + case PeerStatusGood: + m.store.peers[pu.NodeID].MutableScore++ + } +} + // broadcast broadcasts a peer update to all subscriptions. The caller must // already hold the mutex lock, to make sure updates are sent in the same order // as the PeerManager processes them, but this means subscribers must be @@ -837,7 +879,7 @@ func (m *PeerManager) broadcast(peerUpdate PeerUpdate) { default: } select { - case sub.updatesCh <- peerUpdate: + case sub.reactorUpdatesCh <- peerUpdate: case <-sub.closeCh: } } @@ -1149,6 +1191,8 @@ type peerInfo struct { Persistent bool Height int64 FixedScore PeerScore // mainly for tests + + MutableScore int64 // updated by router } // peerInfoFromProto converts a Protobuf PeerInfo message to a peerInfo, @@ -1205,14 +1249,22 @@ func (p *peerInfo) Copy() peerInfo { // Score calculates a score for the peer. Higher-scored peers will be // preferred over lower scores. func (p *peerInfo) Score() PeerScore { - var score PeerScore if p.FixedScore > 0 { return p.FixedScore } if p.Persistent { - score += PeerScorePersistent + return PeerScorePersistent } - return score + + if p.MutableScore <= 0 { + return 0 + } + + if p.MutableScore >= math.MaxUint8 { + return PeerScore(math.MaxUint8) + } + + return PeerScore(p.MutableScore) } // Validate validates the peer info. diff --git a/p2p/peermanager_test.go b/p2p/peermanager_test.go index d15d9f6d6..0c0e4b9bc 100644 --- a/p2p/peermanager_test.go +++ b/p2p/peermanager_test.go @@ -1615,3 +1615,39 @@ func TestPeerManager_SetHeight_GetHeight(t *testing.T) { require.Zero(t, peerManager.GetHeight(a.NodeID)) require.Zero(t, peerManager.GetHeight(b.NodeID)) } + +func TestPeerScoring(t *testing.T) { + // create a mock peer manager + db := dbm.NewMemDB() + peerManager, err := p2p.NewPeerManager(selfID, db, p2p.PeerManagerOptions{}) + require.NoError(t, err) + defer peerManager.Close() + + // create a fake node + id := p2p.NodeID(strings.Repeat("a1", 20)) + require.NoError(t, peerManager.Add(p2p.NodeAddress{NodeID: id, Protocol: "memory"})) + + // update the manager and make sure it's correct + pu := peerManager.Subscribe() + require.EqualValues(t, 0, peerManager.Scores()[id]) + + // add a bunch of good status updates and watch things increase. + for i := 1; i < 10; i++ { + pu.SendUpdate(p2p.PeerUpdate{ + NodeID: id, + Status: p2p.PeerStatusGood, + }) + time.Sleep(time.Millisecond) // force a context switch + require.EqualValues(t, i, peerManager.Scores()[id]) + } + + // watch the corresponding decreases respond to update + for i := 10; i == 0; i-- { + pu.SendUpdate(p2p.PeerUpdate{ + NodeID: id, + Status: p2p.PeerStatusBad, + }) + time.Sleep(time.Millisecond) // force a context switch + require.EqualValues(t, i, peerManager.Scores()[id]) + } +} diff --git a/p2p/shim.go b/p2p/shim.go index 5f20d06dd..5f5e8cede 100644 --- a/p2p/shim.go +++ b/p2p/shim.go @@ -64,7 +64,7 @@ func NewReactorShim(logger log.Logger, name string, descriptors map[ChannelID]*C rs := &ReactorShim{ Name: name, - PeerUpdates: NewPeerUpdates(make(chan PeerUpdate)), + PeerUpdates: NewPeerUpdates(make(chan PeerUpdate), 0), Channels: channels, } @@ -230,7 +230,7 @@ func (rs *ReactorShim) GetChannels() []*ChannelDescriptor { // handle adding a peer. func (rs *ReactorShim) AddPeer(peer Peer) { select { - case rs.PeerUpdates.updatesCh <- PeerUpdate{NodeID: peer.ID(), Status: PeerStatusUp}: + case rs.PeerUpdates.reactorUpdatesCh <- PeerUpdate{NodeID: peer.ID(), Status: PeerStatusUp}: rs.Logger.Debug("sent peer update", "reactor", rs.Name, "peer", peer.ID(), "status", PeerStatusUp) case <-rs.PeerUpdates.Done(): @@ -249,7 +249,7 @@ func (rs *ReactorShim) AddPeer(peer Peer) { // handle removing a peer. func (rs *ReactorShim) RemovePeer(peer Peer, reason interface{}) { select { - case rs.PeerUpdates.updatesCh <- PeerUpdate{NodeID: peer.ID(), Status: PeerStatusDown}: + case rs.PeerUpdates.reactorUpdatesCh <- PeerUpdate{NodeID: peer.ID(), Status: PeerStatusDown}: rs.Logger.Debug( "sent peer update", "reactor", rs.Name, diff --git a/statesync/reactor_test.go b/statesync/reactor_test.go index 0760c1e54..14ffa9509 100644 --- a/statesync/reactor_test.go +++ b/statesync/reactor_test.go @@ -62,7 +62,7 @@ func setup( chunkInCh: make(chan p2p.Envelope, chBuf), chunkOutCh: make(chan p2p.Envelope, chBuf), chunkPeerErrCh: make(chan p2p.PeerError, chBuf), - peerUpdates: p2p.NewPeerUpdates(make(chan p2p.PeerUpdate)), + peerUpdates: p2p.NewPeerUpdates(make(chan p2p.PeerUpdate), int(chBuf)), conn: conn, connQuery: connQuery, stateProvider: stateProvider, From c62e320ffd71f3974ed1e8c1f32f5ed319825b79 Mon Sep 17 00:00:00 2001 From: Sam Kleinman Date: Mon, 29 Mar 2021 17:07:05 -0400 Subject: [PATCH 10/24] p2p: rate-limit incoming connections by IP (#6286) --- p2p/conn_tracker.go | 75 ++++++++++++++++++++++++++++++++++++++++ p2p/conn_tracker_test.go | 73 ++++++++++++++++++++++++++++++++++++++ p2p/router.go | 56 ++++++++++++++++++++++-------- p2p/router_test.go | 3 ++ 4 files changed, 193 insertions(+), 14 deletions(-) create mode 100644 p2p/conn_tracker.go create mode 100644 p2p/conn_tracker_test.go diff --git a/p2p/conn_tracker.go b/p2p/conn_tracker.go new file mode 100644 index 000000000..09673c093 --- /dev/null +++ b/p2p/conn_tracker.go @@ -0,0 +1,75 @@ +package p2p + +import ( + "fmt" + "net" + "sync" + "time" +) + +type connectionTracker interface { + AddConn(net.IP) error + RemoveConn(net.IP) + Len() int +} + +type connTrackerImpl struct { + cache map[string]uint + lastConnect map[string]time.Time + mutex sync.RWMutex + max uint + window time.Duration +} + +func newConnTracker(max uint, window time.Duration) connectionTracker { + return &connTrackerImpl{ + cache: make(map[string]uint), + lastConnect: make(map[string]time.Time), + max: max, + } +} + +func (rat *connTrackerImpl) Len() int { + rat.mutex.RLock() + defer rat.mutex.RUnlock() + return len(rat.cache) +} + +func (rat *connTrackerImpl) AddConn(addr net.IP) error { + address := addr.String() + rat.mutex.Lock() + defer rat.mutex.Unlock() + + if num := rat.cache[address]; num >= rat.max { + return fmt.Errorf("%q has %d connections [max=%d]", address, num, rat.max) + } else if num == 0 { + // if there is already at least connection, check to + // see if it was established before within the window, + // and error if so. + if last := rat.lastConnect[address]; time.Since(last) < rat.window { + return fmt.Errorf("%q tried to connect within window of last %s", address, rat.window) + } + } + + rat.cache[address]++ + rat.lastConnect[address] = time.Now() + + return nil +} + +func (rat *connTrackerImpl) RemoveConn(addr net.IP) { + address := addr.String() + rat.mutex.Lock() + defer rat.mutex.Unlock() + + if num := rat.cache[address]; num > 0 { + rat.cache[address]-- + } + if num := rat.cache[address]; num <= 0 { + delete(rat.cache, address) + } + + if last, ok := rat.lastConnect[address]; ok && time.Since(last) > rat.window { + delete(rat.lastConnect, address) + } +} diff --git a/p2p/conn_tracker_test.go b/p2p/conn_tracker_test.go new file mode 100644 index 000000000..66656e114 --- /dev/null +++ b/p2p/conn_tracker_test.go @@ -0,0 +1,73 @@ +package p2p + +import ( + "math" + "math/rand" + "net" + "testing" + "time" + + "github.com/stretchr/testify/require" +) + +func randByte() byte { + return byte(rand.Intn(math.MaxUint8)) +} + +func randLocalIPv4() net.IP { + return net.IPv4(127, randByte(), randByte(), randByte()) +} + +func TestConnTracker(t *testing.T) { + for name, factory := range map[string]func() connectionTracker{ + "BaseSmall": func() connectionTracker { + return newConnTracker(10, time.Second) + }, + "BaseLarge": func() connectionTracker { + return newConnTracker(100, time.Hour) + }, + } { + t.Run(name, func(t *testing.T) { + factory := factory // nolint:scopelint + t.Run("Initialized", func(t *testing.T) { + ct := factory() + require.Equal(t, 0, ct.Len()) + }) + t.Run("RepeatedAdding", func(t *testing.T) { + ct := factory() + ip := randLocalIPv4() + require.NoError(t, ct.AddConn(ip)) + for i := 0; i < 100; i++ { + _ = ct.AddConn(ip) + } + require.Equal(t, 1, ct.Len()) + }) + t.Run("AddingMany", func(t *testing.T) { + ct := factory() + for i := 0; i < 100; i++ { + _ = ct.AddConn(randLocalIPv4()) + } + require.Equal(t, 100, ct.Len()) + }) + t.Run("Cycle", func(t *testing.T) { + ct := factory() + for i := 0; i < 100; i++ { + ip := randLocalIPv4() + require.NoError(t, ct.AddConn(ip)) + ct.RemoveConn(ip) + } + require.Equal(t, 0, ct.Len()) + }) + }) + } + t.Run("VeryShort", func(t *testing.T) { + ct := newConnTracker(10, time.Microsecond) + for i := 0; i < 10; i++ { + ip := randLocalIPv4() + require.NoError(t, ct.AddConn(ip)) + time.Sleep(2 * time.Microsecond) + require.NoError(t, ct.AddConn(ip)) + } + require.Equal(t, 10, ct.Len()) + }) +} diff --git a/p2p/router.go b/p2p/router.go index beb185a08..2a3235ed6 100644 --- a/p2p/router.go +++ b/p2p/router.go @@ -130,6 +130,15 @@ type RouterOptions struct { // QueueType must be "wdrr" (Weighed Deficit Round Robin), // "priority", or FIFO. Defaults to FIFO. QueueType string + + // MaxIncommingConnectionsPerIP limits the number of incoming + // connections per IP address. Defaults to 100. + MaxIncommingConnectionsPerIP uint + + // IncomingConnectionWindow describes how often an IP address + // can attempt to create a new connection. Defaults to 10 + // milliseconds, and cannot be less than 1 millisecond. + IncomingConnectionWindow time.Duration } const ( @@ -149,6 +158,18 @@ func (o *RouterOptions) Validate() error { return fmt.Errorf("queue type %q is not supported", o.QueueType) } + switch { + case o.IncomingConnectionWindow == 0: + o.IncomingConnectionWindow = 100 * time.Millisecond + case o.IncomingConnectionWindow < time.Millisecond: + return fmt.Errorf("incomming connection window must be grater than 1m [%s]", + o.IncomingConnectionWindow) + } + + if o.MaxIncommingConnectionsPerIP == 0 { + o.MaxIncommingConnectionsPerIP = 100 + } + return nil } @@ -202,6 +223,7 @@ type Router struct { peerManager *PeerManager chDescs []ChannelDescriptor transports []Transport + connTracker connectionTracker protocolTransports map[Protocol]Transport stopCh chan struct{} // signals Router shutdown @@ -235,10 +257,13 @@ func NewRouter( } router := &Router{ - logger: logger, - metrics: metrics, - nodeInfo: nodeInfo, - privKey: privKey, + logger: logger, + metrics: metrics, + nodeInfo: nodeInfo, + privKey: privKey, + connTracker: newConnTracker( + options.MaxIncommingConnectionsPerIP, + options.IncomingConnectionWindow), chDescs: make([]ChannelDescriptor, 0), transports: transports, protocolTransports: map[Protocol]Transport{}, @@ -452,15 +477,6 @@ func (r *Router) acceptPeers(transport Transport) { r.logger.Debug("starting accept routine", "transport", transport) ctx := r.stopCtx() for { - // FIXME: We may need transports to enforce some sort of rate limiting - // here (e.g. by IP address), or alternatively have PeerManager.Accepted() - // do it for us. - // - // FIXME: Even though PeerManager enforces MaxConnected, we may want to - // limit the maximum number of active connections here too, since e.g. - // an adversary can open a ton of connections and then just hang during - // the handshake, taking up TCP socket descriptors. - // // FIXME: The old P2P stack rejected multiple connections for the same IP // unless P2PConfig.AllowDuplicateIP is true -- it's better to limit this // by peer ID rather than IP address, so this hasn't been implemented and @@ -480,9 +496,21 @@ func (r *Router) acceptPeers(transport Transport) { return } + incomingIP := conn.RemoteEndpoint().IP + if err := r.connTracker.AddConn(incomingIP); err != nil { + closeErr := conn.Close() + r.logger.Debug("rate limiting incoming peer", + "err", err, + "ip", incomingIP.String(), + "closeErr", closeErr) + + continue + } + // Spawn a goroutine for the handshake, to avoid head-of-line blocking. go func() { defer conn.Close() + defer r.connTracker.RemoveConn(incomingIP) // FIXME: The peer manager may reject the peer during Accepted() // after we've handshaked with the peer (to find out which peer it @@ -514,7 +542,6 @@ func (r *Router) acceptPeers(transport Transport) { } r.metrics.Peers.Add(1) - queue := r.queueFactory(queueBufferDefault) r.peerMtx.Lock() @@ -692,6 +719,7 @@ func (r *Router) handshakePeer(ctx context.Context, conn Connection, expectID No ctx, cancel = context.WithTimeout(ctx, r.options.HandshakeTimeout) defer cancel() } + peerInfo, peerKey, err := conn.Handshake(ctx, r.nodeInfo, r.privKey) if err != nil { return peerInfo, peerKey, err diff --git a/p2p/router_test.go b/p2p/router_test.go index ac020a7d7..748b4de2e 100644 --- a/p2p/router_test.go +++ b/p2p/router_test.go @@ -334,6 +334,7 @@ func TestRouter_AcceptPeers(t *testing.T) { mockConnection.On("Handshake", mock.Anything, selfInfo, selfKey). Return(tc.peerInfo, tc.peerKey, nil) mockConnection.On("Close").Run(func(_ mock.Arguments) { closer.Close() }).Return(nil) + mockConnection.On("RemoteEndpoint").Return(p2p.Endpoint{}) if tc.ok { mockConnection.On("ReceiveMessage").Return(chID, nil, io.EOF) } @@ -462,6 +463,7 @@ func TestRouter_AcceptPeers_HeadOfLineBlocking(t *testing.T) { mockConnection.On("Handshake", mock.Anything, selfInfo, selfKey). WaitUntil(closeCh).Return(p2p.NodeInfo{}, nil, io.EOF) mockConnection.On("Close").Return(nil) + mockConnection.On("RemoteEndpoint").Return(p2p.Endpoint{}) mockTransport := &mocks.Transport{} mockTransport.On("String").Maybe().Return("mock") @@ -661,6 +663,7 @@ func TestRouter_EvictPeers(t *testing.T) { mockConnection.On("Handshake", mock.Anything, selfInfo, selfKey). Return(peerInfo, peerKey.PubKey(), nil) mockConnection.On("ReceiveMessage").WaitUntil(closeCh).Return(chID, nil, io.EOF) + mockConnection.On("RemoteEndpoint").Return(p2p.Endpoint{}) mockConnection.On("Close").Run(func(_ mock.Arguments) { closeOnce.Do(func() { close(closeCh) From e2dc241c9d26435e5f126c2d55a1e7568c30ae63 Mon Sep 17 00:00:00 2001 From: "dependabot[bot]" <49699333+dependabot[bot]@users.noreply.github.com> Date: Tue, 30 Mar 2021 11:26:13 +0000 Subject: [PATCH 11/24] build(deps): Bump github.com/golang/protobuf from 1.5.1 to 1.5.2 (#6299) Bumps [github.com/golang/protobuf](https://github.com/golang/protobuf) from 1.5.1 to 1.5.2. - [Release notes](https://github.com/golang/protobuf/releases) - [Commits](https://github.com/golang/protobuf/compare/v1.5.1...v1.5.2) Signed-off-by: dependabot[bot] Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> --- go.mod | 2 +- go.sum | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/go.mod b/go.mod index 1afc2d4a5..f2aba23b3 100644 --- a/go.mod +++ b/go.mod @@ -14,7 +14,7 @@ require ( github.com/go-kit/kit v0.10.0 github.com/go-logfmt/logfmt v0.5.0 github.com/gogo/protobuf v1.3.2 - github.com/golang/protobuf v1.5.1 + github.com/golang/protobuf v1.5.2 github.com/google/orderedcode v0.0.1 github.com/gorilla/websocket v1.4.2 github.com/grpc-ecosystem/go-grpc-middleware v1.2.2 diff --git a/go.sum b/go.sum index 46ca7c349..6e026716e 100644 --- a/go.sum +++ b/go.sum @@ -187,8 +187,8 @@ github.com/golang/protobuf v1.4.1/go.mod h1:U8fpvMrcmy5pZrNK1lt4xCsGvpyWQ/VVv6QD github.com/golang/protobuf v1.4.2/go.mod h1:oDoupMAO8OvCJWAcko0GGGIgR6R6ocIYbsSw735rRwI= github.com/golang/protobuf v1.4.3/go.mod h1:oDoupMAO8OvCJWAcko0GGGIgR6R6ocIYbsSw735rRwI= github.com/golang/protobuf v1.5.0/go.mod h1:FsONVRAS9T7sI+LIUmWTfcYkHO4aIWwzhcaSAoJOfIk= -github.com/golang/protobuf v1.5.1 h1:jAbXjIeW2ZSW2AwFxlGTDoc2CjI2XujLkV3ArsZFCvc= -github.com/golang/protobuf v1.5.1/go.mod h1:DopwsBzvsk0Fs44TXzsVbJyPhcCPeIwnvohx4u74HPM= +github.com/golang/protobuf v1.5.2 h1:ROPKBNFfQgOUMifHyP+KYbvpjbdoFNs+aK7DXlji0Tw= +github.com/golang/protobuf v1.5.2/go.mod h1:XVQd3VNwM+JqD3oG2Ue2ip4fOMUkwXdXDdiuN0vRsmY= github.com/golang/snappy v0.0.0-20180518054509-2e65f85255db/go.mod h1:/XxbfmMg8lxefKM7IXC3fBNl/7bRcc72aCRzEWrmP2Q= github.com/golang/snappy v0.0.1 h1:Qgr9rKW7uDUkrbSmQeiDsGa8SjGyCOGtuasMWwvp2P4= github.com/golang/snappy v0.0.1/go.mod h1:/XxbfmMg8lxefKM7IXC3fBNl/7bRcc72aCRzEWrmP2Q= From 70ea675f3849a805ca937025b1402cae09645897 Mon Sep 17 00:00:00 2001 From: "dependabot[bot]" <49699333+dependabot[bot]@users.noreply.github.com> Date: Tue, 30 Mar 2021 11:41:47 +0000 Subject: [PATCH 12/24] build(deps): Bump github.com/Workiva/go-datastructures (#6298) Bumps [github.com/Workiva/go-datastructures](https://github.com/Workiva/go-datastructures) from 1.0.52 to 1.0.53. - [Release notes](https://github.com/Workiva/go-datastructures/releases) - [Commits](https://github.com/Workiva/go-datastructures/compare/v1.0.52...v1.0.53) Signed-off-by: dependabot[bot] Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> Co-authored-by: Marko --- go.mod | 2 +- go.sum | 7 ++++++- 2 files changed, 7 insertions(+), 2 deletions(-) diff --git a/go.mod b/go.mod index f2aba23b3..f29a4591d 100644 --- a/go.mod +++ b/go.mod @@ -5,7 +5,7 @@ go 1.15 require ( github.com/BurntSushi/toml v0.3.1 github.com/ChainSafe/go-schnorrkel v0.0.0-20210222182958-bd440c890782 - github.com/Workiva/go-datastructures v1.0.52 + github.com/Workiva/go-datastructures v1.0.53 github.com/btcsuite/btcd v0.21.0-beta github.com/btcsuite/btcutil v1.0.2 github.com/confio/ics23/go v0.6.3 diff --git a/go.sum b/go.sum index 6e026716e..b5cd881cf 100644 --- a/go.sum +++ b/go.sum @@ -28,8 +28,9 @@ github.com/Shopify/sarama v1.19.0/go.mod h1:FVkBWblsNy7DGZRfXLU0O9RCGt5g3g3yEuWX github.com/Shopify/toxiproxy v2.1.4+incompatible/go.mod h1:OXgGpZ6Cli1/URJOF1DMxUHB2q5Ap20/P/eIdh4G0pI= github.com/VividCortex/gohistogram v1.0.0 h1:6+hBz+qvs0JOrrNhhmR7lFxo5sINxBCGXrdtl/UvroE= github.com/VividCortex/gohistogram v1.0.0/go.mod h1:Pf5mBqqDxYaXu3hDrrU+w6nw50o/4+TcAqDqk/vUH7g= -github.com/Workiva/go-datastructures v1.0.52 h1:PLSK6pwn8mYdaoaCZEMsXBpBotr4HHn9abU0yMQt0NI= github.com/Workiva/go-datastructures v1.0.52/go.mod h1:Z+F2Rca0qCsVYDS8z7bAGm8f3UkzuWYS/oBZz5a7VVA= +github.com/Workiva/go-datastructures v1.0.53 h1:J6Y/52yX10Xc5JjXmGtWoSSxs3mZnGSaq37xZZh7Yig= +github.com/Workiva/go-datastructures v1.0.53/go.mod h1:1yZL+zfsztete+ePzZz/Zb1/t5BnDuE2Ya2MMGhzP6A= github.com/aead/siphash v1.0.1/go.mod h1:Nywa3cDsYNNK3gaciGTWPwHt0wlpNV15vwmswBAUSII= github.com/afex/hystrix-go v0.0.0-20180502004556-fa1af6a1f4f5/go.mod h1:SkGFH1ia65gfNATL8TAiHDNxPzPdmEL5uirI2Uyuz6c= github.com/alecthomas/template v0.0.0-20160405071501-a0175ee3bccc/go.mod h1:LOuyumcjzFXgccqObfd/Ljyb9UuFJ6TxHnclSeseNhc= @@ -378,6 +379,7 @@ github.com/pelletier/go-toml v1.2.0/go.mod h1:5z9KED0ma1S8pY6P1sdut58dfprrGBbd/9 github.com/performancecopilot/speed v3.0.0+incompatible/go.mod h1:/CLtqpZ5gBg1M9iaPbIdPPGyKcA8hKdoy6hAWba7Yac= github.com/petermattis/goid v0.0.0-20180202154549-b0b1615b78e5 h1:q2e307iGHPdTGp0hoxKjt1H5pDo6utceo3dQVK3I5XQ= github.com/petermattis/goid v0.0.0-20180202154549-b0b1615b78e5/go.mod h1:jvVRKCrJTQWu0XVbaOlby/2lO20uSCHEMzzplHXte1o= +github.com/philhofer/fwd v1.1.1/go.mod h1:gk3iGcWd9+svBvR0sR+KPcfE+RNWozjowpeBVG3ZVNU= github.com/pierrec/lz4 v1.0.2-0.20190131084431-473cd7ce01a1/go.mod h1:3/3N9NVKO0jef7pBehbT1qWhCMrIgbYNnFAZCqQ5LRc= github.com/pierrec/lz4 v2.0.5+incompatible/go.mod h1:pdkljMzZIN41W+lC3N2tnIh5sFi+IEE17M5jbnwPHcY= github.com/pkg/errors v0.8.0/go.mod h1:bwawxfHBFNV+L2hUp1rHADufV3IMtnDRdf1r5NINEl0= @@ -501,8 +503,10 @@ github.com/tendermint/tm-db v0.6.2/go.mod h1:GYtQ67SUvATOcoY8/+x6ylk8Qo02BQyLrAs github.com/tendermint/tm-db v0.6.3/go.mod h1:lfA1dL9/Y/Y8wwyPp2NMLyn5P5Ptr/gvDFNWtrCWSf8= github.com/tendermint/tm-db v0.6.4 h1:3N2jlnYQkXNQclQwd/eKV/NzlqPlfK21cpRRIx80XXQ= github.com/tendermint/tm-db v0.6.4/go.mod h1:dptYhIpJ2M5kUuenLr+Yyf3zQOv1SgBZcl8/BmWlMBw= +github.com/tinylib/msgp v1.1.5/go.mod h1:eQsjooMTnV42mHu917E26IogZ2930nFyBQdofk10Udg= github.com/tmc/grpc-websocket-proxy v0.0.0-20170815181823-89b8d40f7ca8/go.mod h1:ncp9v5uamzpCO7NfCPTXjqaC+bZgJeR0sMTm6dMHP7U= github.com/tmc/grpc-websocket-proxy v0.0.0-20190109142713-0ad062ec5ee5/go.mod h1:ncp9v5uamzpCO7NfCPTXjqaC+bZgJeR0sMTm6dMHP7U= +github.com/ttacon/chalk v0.0.0-20160626202418-22c06c80ed31/go.mod h1:onvgF043R+lC5RZ8IT9rBXDaEDnpnw/Cl+HFiw+v/7Q= github.com/ugorji/go v1.1.4/go.mod h1:uQMGLiO92mf5W77hV/PUCpI3pbzQx3CRekS0kk+RGrc= github.com/ugorji/go/codec v0.0.0-20181204163529-d75b2dcb6bc8/go.mod h1:VFNgLljTbGfSG7qAOspJ7OScBnGdDN/yBr0sguwnwf0= github.com/urfave/cli v1.20.0/go.mod h1:70zkFmudgCuE/ngEzBv17Jvp/497gISqfk5gWijbERA= @@ -686,6 +690,7 @@ golang.org/x/tools v0.0.0-20191119224855-298f0cb1881e/go.mod h1:b+2E5dAYhXwXZwtn golang.org/x/tools v0.0.0-20200103221440-774c71fcf114/go.mod h1:TB2adYChydJhpapKDTa4BR/hXlZSLoq2Wpct/0txZ28= golang.org/x/tools v0.0.0-20200207183749-b753a1ba74fa/go.mod h1:TB2adYChydJhpapKDTa4BR/hXlZSLoq2Wpct/0txZ28= golang.org/x/tools v0.0.0-20200619180055-7c47624df98f/go.mod h1:EkVYQZoAsY45+roYkvgYkIh4xh/qjgUK9TdY2XT94GE= +golang.org/x/tools v0.0.0-20201022035929-9cf592e881e9/go.mod h1:emZCQorbCU4vsT4fOWvOPXz4eW1wZW4PmDk9uLelYpA= golang.org/x/tools v0.0.0-20210106214847-113979e3529a/go.mod h1:emZCQorbCU4vsT4fOWvOPXz4eW1wZW4PmDk9uLelYpA= golang.org/x/xerrors v0.0.0-20190717185122-a985d3407aa7/go.mod h1:I/5z698sn9Ka8TeJc9MKroUUfqBBauWjQqLJ2OPfmY0= golang.org/x/xerrors v0.0.0-20191011141410-1b5146add898/go.mod h1:I/5z698sn9Ka8TeJc9MKroUUfqBBauWjQqLJ2OPfmY0= From 3ed8f14bf973bb8cc3c262240a291f07d45b3f3f Mon Sep 17 00:00:00 2001 From: Sam Kleinman Date: Tue, 30 Mar 2021 10:04:14 -0400 Subject: [PATCH 13/24] p2p: connect max inbound peers configuration to new router (#6296) --- node/node.go | 21 ++++++++++++++++++--- p2p/router.go | 5 ----- 2 files changed, 18 insertions(+), 8 deletions(-) diff --git a/node/node.go b/node/node.go index 2c3a713f0..9304bf41c 100644 --- a/node/node.go +++ b/node/node.go @@ -640,6 +640,7 @@ func createRouter( privKey crypto.PrivKey, peerManager *p2p.PeerManager, transport p2p.Transport, + options p2p.RouterOptions, ) (*p2p.Router, error) { return p2p.NewRouter( @@ -649,7 +650,7 @@ func createRouter( privKey, peerManager, []p2p.Transport{transport}, - p2p.RouterOptions{QueueType: p2pRouterQueueType}, + options, ) } @@ -915,7 +916,8 @@ func NewSeedNode(config *cfg.Config, return nil, fmt.Errorf("failed to create peer manager: %w", err) } - router, err := createRouter(p2pLogger, p2pMetrics, nodeInfo, nodeKey.PrivKey, peerManager, transport) + router, err := createRouter(p2pLogger, p2pMetrics, nodeInfo, nodeKey.PrivKey, + peerManager, transport, getRouterConfig(config)) if err != nil { return nil, fmt.Errorf("failed to create router: %w", err) } @@ -1077,7 +1079,8 @@ func NewNode(config *cfg.Config, csMetrics, p2pMetrics, memplMetrics, smMetrics := metricsProvider(genDoc.ChainID) - router, err := createRouter(p2pLogger, p2pMetrics, nodeInfo, nodeKey.PrivKey, peerManager, transport) + router, err := createRouter(p2pLogger, p2pMetrics, nodeInfo, nodeKey.PrivKey, + peerManager, transport, getRouterConfig(config)) if err != nil { return nil, fmt.Errorf("failed to create router: %w", err) } @@ -1960,6 +1963,18 @@ func createAndStartPrivValidatorGRPCClient( return pvsc, nil } +func getRouterConfig(conf *cfg.Config) p2p.RouterOptions { + opts := p2p.RouterOptions{ + QueueType: p2pRouterQueueType, + } + + if conf.P2P.MaxNumInboundPeers > 0 { + opts.MaxIncommingConnectionsPerIP = uint(conf.P2P.MaxNumInboundPeers) + } + + return opts +} + // FIXME: Temporary helper function, shims should be removed. func makeChannelsFromShims( router *p2p.Router, diff --git a/p2p/router.go b/p2p/router.go index 2a3235ed6..8fbeccc8b 100644 --- a/p2p/router.go +++ b/p2p/router.go @@ -477,11 +477,6 @@ func (r *Router) acceptPeers(transport Transport) { r.logger.Debug("starting accept routine", "transport", transport) ctx := r.stopCtx() for { - // FIXME: The old P2P stack rejected multiple connections for the same IP - // unless P2PConfig.AllowDuplicateIP is true -- it's better to limit this - // by peer ID rather than IP address, so this hasn't been implemented and - // probably shouldn't (?). - // // FIXME: The old P2P stack supported ABCI-based IP address filtering via // /p2p/filter/addr/ queries, do we want to implement this here as well? // Filtering by node ID is probably better. From d0b513c182ee777ffed3bef983e07981416f9a36 Mon Sep 17 00:00:00 2001 From: Sam Kleinman Date: Tue, 30 Mar 2021 18:00:43 -0400 Subject: [PATCH 14/24] p2p: filter peers by IP address and ID (#6300) --- node/node.go | 37 +++++++- p2p/router.go | 177 ++++++++++++++++++++++++-------------- p2p/router_filter_test.go | 34 ++++++++ 3 files changed, 181 insertions(+), 67 deletions(-) create mode 100644 p2p/router_filter_test.go diff --git a/node/node.go b/node/node.go index 9304bf41c..4b2b584ae 100644 --- a/node/node.go +++ b/node/node.go @@ -917,7 +917,7 @@ func NewSeedNode(config *cfg.Config, } router, err := createRouter(p2pLogger, p2pMetrics, nodeInfo, nodeKey.PrivKey, - peerManager, transport, getRouterConfig(config)) + peerManager, transport, getRouterConfig(config, nil)) if err != nil { return nil, fmt.Errorf("failed to create router: %w", err) } @@ -1080,7 +1080,7 @@ func NewNode(config *cfg.Config, csMetrics, p2pMetrics, memplMetrics, smMetrics := metricsProvider(genDoc.ChainID) router, err := createRouter(p2pLogger, p2pMetrics, nodeInfo, nodeKey.PrivKey, - peerManager, transport, getRouterConfig(config)) + peerManager, transport, getRouterConfig(config, proxyApp)) if err != nil { return nil, fmt.Errorf("failed to create router: %w", err) } @@ -1963,7 +1963,7 @@ func createAndStartPrivValidatorGRPCClient( return pvsc, nil } -func getRouterConfig(conf *cfg.Config) p2p.RouterOptions { +func getRouterConfig(conf *cfg.Config, proxyApp proxy.AppConns) p2p.RouterOptions { opts := p2p.RouterOptions{ QueueType: p2pRouterQueueType, } @@ -1972,6 +1972,37 @@ func getRouterConfig(conf *cfg.Config) p2p.RouterOptions { opts.MaxIncommingConnectionsPerIP = uint(conf.P2P.MaxNumInboundPeers) } + if conf.FilterPeers && proxyApp != nil { + opts.FilterPeerByID = func(ctx context.Context, id p2p.NodeID) error { + res, err := proxyApp.Query().QuerySync(context.Background(), abci.RequestQuery{ + Path: fmt.Sprintf("/p2p/filter/id/%s", id), + }) + if err != nil { + return err + } + if res.IsErr() { + return fmt.Errorf("error querying abci app: %v", res) + } + + return nil + } + + opts.FilterPeerByIP = func(ctx context.Context, ip net.IP, port uint16) error { + res, err := proxyApp.Query().QuerySync(ctx, abci.RequestQuery{ + Path: fmt.Sprintf("/p2p/filter/addr/%s", net.JoinHostPort(ip.String(), strconv.Itoa(int(port)))), + }) + if err != nil { + return err + } + if res.IsErr() { + return fmt.Errorf("error querying abci app: %v", res) + } + + return nil + } + + } + return opts } diff --git a/p2p/router.go b/p2p/router.go index 8fbeccc8b..5c55164f3 100644 --- a/p2p/router.go +++ b/p2p/router.go @@ -5,6 +5,7 @@ import ( "errors" "fmt" "io" + "net" "sync" "time" @@ -139,6 +140,21 @@ type RouterOptions struct { // can attempt to create a new connection. Defaults to 10 // milliseconds, and cannot be less than 1 millisecond. IncomingConnectionWindow time.Duration + + // FilterPeerByIP is used by the router to inject filtering + // behavior for new incoming connections. The router passes + // the remote IP of the incoming connection the port number as + // arguments. Functions should return an error to reject the + // peer. + FilterPeerByIP func(context.Context, net.IP, uint16) error + + // FilterPeerByID is used by the router to inject filtering + // behavior for new incoming connections. The router passes + // the NodeID of the node before completing the connection, + // but this occurs after the handshake is complete. Filter by + // IP address to filter before the handshake. Functions should + // return an error to reject the peer. + FilterPeerByID func(context.Context, NodeID) error } const ( @@ -471,15 +487,28 @@ func (r *Router) routeChannel( } } +func (r *Router) filterPeersIP(ctx context.Context, ip net.IP, port uint16) error { + if r.options.FilterPeerByIP == nil { + return nil + } + + return r.options.FilterPeerByIP(ctx, ip, port) +} + +func (r *Router) filterPeersID(ctx context.Context, id NodeID) error { + if r.options.FilterPeerByID == nil { + return nil + } + + return r.options.FilterPeerByID(ctx, id) +} + // acceptPeers accepts inbound connections from peers on the given transport, // and spawns goroutines that route messages to/from them. func (r *Router) acceptPeers(transport Transport) { r.logger.Debug("starting accept routine", "transport", transport) ctx := r.stopCtx() for { - // FIXME: The old P2P stack supported ABCI-based IP address filtering via - // /p2p/filter/addr/ queries, do we want to implement this here as well? - // Filtering by node ID is probably better. conn, err := transport.Accept() switch err { case nil: @@ -499,74 +528,94 @@ func (r *Router) acceptPeers(transport Transport) { "ip", incomingIP.String(), "closeErr", closeErr) - continue + return } // Spawn a goroutine for the handshake, to avoid head-of-line blocking. - go func() { - defer conn.Close() - defer r.connTracker.RemoveConn(incomingIP) + go r.openConnection(ctx, conn) - // FIXME: The peer manager may reject the peer during Accepted() - // after we've handshaked with the peer (to find out which peer it - // is). However, because the handshake has no ack, the remote peer - // will think the handshake was successful and start sending us - // messages. - // - // This can cause problems in tests, where a disconnection can cause - // the local node to immediately redial, while the remote node may - // not have completed the disconnection yet and therefore reject the - // reconnection attempt (since it thinks we're still connected from - // before). - // - // The Router should do the handshake and have a final ack/fail - // message to make sure both ends have accepted the connection, such - // that it can be coordinated with the peer manager. - peerInfo, _, err := r.handshakePeer(ctx, conn, "") - switch { - case errors.Is(err, context.Canceled): - return - case err != nil: - r.logger.Error("peer handshake failed", "endpoint", conn, "err", err) - return - } - - if err := r.peerManager.Accepted(peerInfo.NodeID); err != nil { - r.logger.Error("failed to accept connection", "peer", peerInfo.NodeID, "err", err) - return - } - - r.metrics.Peers.Add(1) - queue := r.queueFactory(queueBufferDefault) - - r.peerMtx.Lock() - r.peerQueues[peerInfo.NodeID] = queue - r.peerMtx.Unlock() - - defer func() { - r.peerMtx.Lock() - delete(r.peerQueues, peerInfo.NodeID) - r.peerMtx.Unlock() - - queue.close() - - if err := r.peerManager.Disconnected(peerInfo.NodeID); err != nil { - r.logger.Error("failed to disconnect peer", "peer", peerInfo.NodeID, "err", err) - } else { - r.metrics.Peers.Add(-1) - } - }() - - if err := r.peerManager.Ready(peerInfo.NodeID); err != nil { - r.logger.Error("failed to mark peer as ready", "peer", peerInfo.NodeID, "err", err) - return - } - - r.routePeer(peerInfo.NodeID, conn, queue) - }() } } +func (r *Router) openConnection(ctx context.Context, conn Connection) { + defer conn.Close() + defer r.connTracker.RemoveConn(conn.RemoteEndpoint().IP) + + re := conn.RemoteEndpoint() + incomingIP := re.IP + + if err := r.filterPeersIP(ctx, incomingIP, re.Port); err != nil { + r.logger.Debug("peer filtered by IP", + "ip", incomingIP.String(), + "err", err) + return + } + + // FIXME: The peer manager may reject the peer during Accepted() + // after we've handshaked with the peer (to find out which peer it + // is). However, because the handshake has no ack, the remote peer + // will think the handshake was successful and start sending us + // messages. + // + // This can cause problems in tests, where a disconnection can cause + // the local node to immediately redial, while the remote node may + // not have completed the disconnection yet and therefore reject the + // reconnection attempt (since it thinks we're still connected from + // before). + // + // The Router should do the handshake and have a final ack/fail + // message to make sure both ends have accepted the connection, such + // that it can be coordinated with the peer manager. + peerInfo, _, err := r.handshakePeer(ctx, conn, "") + switch { + case errors.Is(err, context.Canceled): + return + case err != nil: + r.logger.Error("peer handshake failed", "endpoint", conn, "err", err) + return + } + + if err := r.filterPeersID(ctx, peerInfo.NodeID); err != nil { + r.logger.Debug("peer filtered by node ID", + "node", peerInfo.NodeID, + "err", err) + return + } + + if err := r.peerManager.Accepted(peerInfo.NodeID); err != nil { + r.logger.Error("failed to accept connection", "peer", peerInfo.NodeID, "err", err) + return + } + + r.metrics.Peers.Add(1) + queue := r.queueFactory(queueBufferDefault) + + r.peerMtx.Lock() + r.peerQueues[peerInfo.NodeID] = queue + r.peerMtx.Unlock() + + defer func() { + r.peerMtx.Lock() + delete(r.peerQueues, peerInfo.NodeID) + r.peerMtx.Unlock() + + queue.close() + + if err := r.peerManager.Disconnected(peerInfo.NodeID); err != nil { + r.logger.Error("failed to disconnect peer", "peer", peerInfo.NodeID, "err", err) + } else { + r.metrics.Peers.Add(-1) + } + }() + + if err := r.peerManager.Ready(peerInfo.NodeID); err != nil { + r.logger.Error("failed to mark peer as ready", "peer", peerInfo.NodeID, "err", err) + return + } + + r.routePeer(peerInfo.NodeID, conn, queue) +} + // dialPeers maintains outbound connections to peers by dialing them. func (r *Router) dialPeers() { r.logger.Debug("starting dial routine") diff --git a/p2p/router_filter_test.go b/p2p/router_filter_test.go new file mode 100644 index 000000000..51c4947b4 --- /dev/null +++ b/p2p/router_filter_test.go @@ -0,0 +1,34 @@ +package p2p + +import ( + "context" + "errors" + "net" + "testing" + "time" + + "github.com/stretchr/testify/require" + "github.com/tendermint/tendermint/libs/log" + "github.com/tendermint/tendermint/libs/sync" +) + +func TestConnectionFiltering(t *testing.T) { + ctx, cancel := context.WithCancel(context.Background()) + defer cancel() + logger := log.TestingLogger() + + filterByIPCount := 0 + router := &Router{ + logger: logger, + connTracker: newConnTracker(1, time.Second), + options: RouterOptions{ + FilterPeerByIP: func(ctx context.Context, ip net.IP, port uint16) error { + filterByIPCount++ + return errors.New("mock") + }, + }, + } + require.Equal(t, 0, filterByIPCount) + router.openConnection(ctx, &MemoryConnection{logger: logger, closer: sync.NewCloser()}) + require.Equal(t, 1, filterByIPCount) +} From 0811c7be9909d590987e88a3e8398f27c5d3d44f Mon Sep 17 00:00:00 2001 From: Sam Kleinman Date: Thu, 1 Apr 2021 10:03:51 -0400 Subject: [PATCH 15/24] fix: theoretical leak in clisit.Init (#6302) --- libs/clist/clist.go | 21 ++++++++------------- 1 file changed, 8 insertions(+), 13 deletions(-) diff --git a/libs/clist/clist.go b/libs/clist/clist.go index 5579b1d0f..a32098774 100644 --- a/libs/clist/clist.go +++ b/libs/clist/clist.go @@ -229,18 +229,6 @@ type CList struct { maxLen int // max list length } -func (l *CList) Init() *CList { - l.mtx.Lock() - - l.wg = waitGroup1() - l.waitCh = make(chan struct{}) - l.head = nil - l.tail = nil - l.len = 0 - l.mtx.Unlock() - return l -} - // Return CList with MaxLength. CList will panic if it goes beyond MaxLength. func New() *CList { return newWithMax(MaxLength) } @@ -249,7 +237,14 @@ func New() *CList { return newWithMax(MaxLength) } func newWithMax(maxLength int) *CList { l := new(CList) l.maxLen = maxLength - return l.Init() + + l.wg = waitGroup1() + l.waitCh = make(chan struct{}) + l.head = nil + l.tail = nil + l.len = 0 + + return l } func (l *CList) Len() int { From 19393f0c28d0004115aea32f89d6237c7b7e0468 Mon Sep 17 00:00:00 2001 From: Sam Kleinman Date: Thu, 1 Apr 2021 11:24:23 -0400 Subject: [PATCH 16/24] test: clean up databases in tests (#6304) --- abci/example/kvstore/persistent_kvstore.go | 4 ++++ consensus/replay_test.go | 9 ++++++--- consensus/wal_generator.go | 2 ++ 3 files changed, 12 insertions(+), 3 deletions(-) diff --git a/abci/example/kvstore/persistent_kvstore.go b/abci/example/kvstore/persistent_kvstore.go index d40983f85..0fcfcadf7 100644 --- a/abci/example/kvstore/persistent_kvstore.go +++ b/abci/example/kvstore/persistent_kvstore.go @@ -51,6 +51,10 @@ func NewPersistentKVStoreApplication(dbDir string) *PersistentKVStoreApplication } } +func (app *PersistentKVStoreApplication) Close() error { + return app.app.state.db.Close() +} + func (app *PersistentKVStoreApplication) SetLogger(l log.Logger) { app.logger = l } diff --git a/consensus/replay_test.go b/consensus/replay_test.go index 0a7430ab8..8175033ab 100644 --- a/consensus/replay_test.go +++ b/consensus/replay_test.go @@ -704,6 +704,7 @@ func testHandshakeReplay(t *testing.T, config *cfg.Config, nBlocks int, mode uin // make a new client creator kvstoreApp := kvstore.NewPersistentKVStoreApplication( filepath.Join(config.DBDir(), fmt.Sprintf("replay_test_%d_%d_a", nBlocks, mode))) + t.Cleanup(func() { require.NoError(t, kvstoreApp.Close()) }) clientCreator2 := proxy.NewLocalClientCreator(kvstoreApp) if nBlocks > 0 { @@ -835,9 +836,11 @@ func buildTMStateFromChain( nBlocks int, mode uint) sm.State { // run the whole chain against this client to build up the tendermint state - clientCreator := proxy.NewLocalClientCreator( - kvstore.NewPersistentKVStoreApplication( - filepath.Join(config.DBDir(), fmt.Sprintf("replay_test_%d_%d_t", nBlocks, mode)))) + kvstoreApp := kvstore.NewPersistentKVStoreApplication( + filepath.Join(config.DBDir(), fmt.Sprintf("replay_test_%d_%d_t", nBlocks, mode))) + defer kvstoreApp.Close() + clientCreator := proxy.NewLocalClientCreator(kvstoreApp) + proxyApp := proxy.NewAppConns(clientCreator) if err := proxyApp.Start(); err != nil { panic(err) diff --git a/consensus/wal_generator.go b/consensus/wal_generator.go index e82bfe8a2..bbb31f5ad 100644 --- a/consensus/wal_generator.go +++ b/consensus/wal_generator.go @@ -9,6 +9,7 @@ import ( "testing" "time" + "github.com/stretchr/testify/require" db "github.com/tendermint/tm-db" "github.com/tendermint/tendermint/abci/example/kvstore" @@ -31,6 +32,7 @@ func WALGenerateNBlocks(t *testing.T, wr io.Writer, numBlocks int) (err error) { config := getConfig(t) app := kvstore.NewPersistentKVStoreApplication(filepath.Join(config.DBDir(), "wal_generator")) + t.Cleanup(func() { require.NoError(t, app.Close()) }) logger := log.TestingLogger().With("wal_generator", "wal_generator") logger.Info("generating WAL (last height msg excluded)", "numBlocks", numBlocks) From f2f085c7a3ecf69ed4e7249d2fe1765be47f8ef1 Mon Sep 17 00:00:00 2001 From: Sam Kleinman Date: Thu, 1 Apr 2021 13:01:04 -0400 Subject: [PATCH 17/24] fix: test fixture peer manager in mempool reactor tests (#6308) --- mempool/reactor_test.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/mempool/reactor_test.go b/mempool/reactor_test.go index e287302f0..f9c609dac 100644 --- a/mempool/reactor_test.go +++ b/mempool/reactor_test.go @@ -66,7 +66,7 @@ func setup(t *testing.T, cfg *cfg.MempoolConfig, numNodes int, chBuf uint) *reac rts.reactors[nodeID] = NewReactor( rts.logger.With("nodeID", nodeID), cfg, - rts.network.RandomNode().PeerManager, + rts.network.Nodes[nodeID].PeerManager, mempool, rts.mempoolChnnels[nodeID], rts.peerUpdates[nodeID], From 358d1a28b8c126ce2a4b88fa675096a296410de0 Mon Sep 17 00:00:00 2001 From: Callum Waters Date: Fri, 2 Apr 2021 09:31:25 +0200 Subject: [PATCH 18/24] node: remove mode defaults. Make node mode explicit (#6282) --- .gitignore | 1 + CHANGELOG_PENDING.md | 1 + DOCKER/docker-entrypoint.sh | 2 +- UPGRADING.md | 4 +- cmd/tendermint/commands/init.go | 74 ++++++++++++-------- cmd/tendermint/commands/testnet.go | 5 +- config/config.go | 17 +++-- config/toml.go | 39 +++++------ config/toml_test.go | 2 + docs/app-dev/getting-started.md | 2 +- docs/architecture/adr-008-priv-validator.md | 2 +- docs/architecture/adr-052-tendermint-mode.md | 6 +- docs/introduction/install.md | 2 +- docs/introduction/quick-start.md | 2 +- docs/nodes/configuration.md | 13 ++-- docs/tendermint-core/using-tendermint.md | 2 +- docs/tools/remote-signer-validation.md | 2 +- docs/tutorials/go-built-in.md | 9 +-- docs/tutorials/go.md | 5 +- docs/tutorials/java.md | 3 +- docs/tutorials/kotlin.md | 5 +- test/app/test.sh | 10 +-- test/e2e/runner/setup.go | 2 +- 23 files changed, 120 insertions(+), 90 deletions(-) diff --git a/.gitignore b/.gitignore index b2a29d086..6eb5b9675 100644 --- a/.gitignore +++ b/.gitignore @@ -35,6 +35,7 @@ shunit2 terraform.tfstate terraform.tfstate.backup terraform.tfstate.d +test/app/grpc_client test/e2e/build test/e2e/networks/*/ test/logs diff --git a/CHANGELOG_PENDING.md b/CHANGELOG_PENDING.md index da3fdd837..92af3dc30 100644 --- a/CHANGELOG_PENDING.md +++ b/CHANGELOG_PENDING.md @@ -15,6 +15,7 @@ Friendly reminder: We have a [bug bounty program](https://hackerone.com/tendermi - [cli] \#5777 use hyphen-case instead of snake_case for all cli commands and config parameters (@cmwaters) - [rpc] \#6019 standardise RPC errors and return the correct status code (@bipulprasad & @cmwaters) - [rpc] \#6168 Change default sorting to desc for `/tx_search` results (@melekes) + - [cli] \#6282 User must specify the node mode when using `tendermint init` (@cmwaters) - Apps - [ABCI] \#5447 Remove `SetOption` method from `ABCI.Client` interface diff --git a/DOCKER/docker-entrypoint.sh b/DOCKER/docker-entrypoint.sh index d74511c17..e6442b485 100755 --- a/DOCKER/docker-entrypoint.sh +++ b/DOCKER/docker-entrypoint.sh @@ -3,7 +3,7 @@ set -e if [ ! -d "$TMHOME/config" ]; then echo "Running tendermint init to create (default) configuration for docker run." - tendermint init + tendermint init validator sed -i \ -e "s/^proxy-app\s*=.*/proxy-app = \"$PROXY_APP\"/" \ diff --git a/UPGRADING.md b/UPGRADING.md index ce8c2a2a6..e9170ffab 100644 --- a/UPGRADING.md +++ b/UPGRADING.md @@ -24,9 +24,11 @@ This guide provides instructions for upgrading to specific versions of Tendermin ### CLI Changes +* You must now specify the node mode (validator|full|seed) in `tendermint init [mode]` + * If you had previously used `tendermint gen_node_key` to generate a new node key, keep in mind that it no longer saves the output to a file. You can use - `tendermint init` or pipe the output of `tendermint gen_node_key` to + `tendermint init validator` or pipe the output of `tendermint gen_node_key` to `$TMHOME/config/node_key.json`: ``` diff --git a/cmd/tendermint/commands/init.go b/cmd/tendermint/commands/init.go index efe0f0244..99a59773c 100644 --- a/cmd/tendermint/commands/init.go +++ b/cmd/tendermint/commands/init.go @@ -2,6 +2,7 @@ package commands import ( "context" + "errors" "fmt" "github.com/spf13/cobra" @@ -17,9 +18,12 @@ import ( // InitFilesCmd initializes a fresh Tendermint Core instance. var InitFilesCmd = &cobra.Command{ - Use: "init", - Short: "Initialize Tendermint", - RunE: initFiles, + Use: "init [full|validator|seed]", + Short: "Initializes a Tendermint node", + ValidArgs: []string{"full", "validator", "seed"}, + // We allow for zero args so we can throw a more informative error + Args: cobra.MaximumNArgs(1), + RunE: initFiles, } var ( @@ -32,33 +36,40 @@ func init() { } func initFiles(cmd *cobra.Command, args []string) error { + if len(args) == 0 { + return errors.New("must specify a node type: tendermint init [validator|full|seed]") + } + config.Mode = args[0] return initFilesWithConfig(config) } func initFilesWithConfig(config *cfg.Config) error { - // private validator - privValKeyFile := config.PrivValidatorKeyFile() - privValStateFile := config.PrivValidatorStateFile() var ( pv *privval.FilePV err error ) - if tmos.FileExists(privValKeyFile) { - pv, err = privval.LoadFilePV(privValKeyFile, privValStateFile) - if err != nil { - return err - } - logger.Info("Found private validator", "keyFile", privValKeyFile, - "stateFile", privValStateFile) - } else { - pv, err = privval.GenFilePV(privValKeyFile, privValStateFile, keyType) - if err != nil { - return err + if config.Mode == cfg.ModeValidator { + // private validator + privValKeyFile := config.PrivValidatorKeyFile() + privValStateFile := config.PrivValidatorStateFile() + if tmos.FileExists(privValKeyFile) { + pv, err = privval.LoadFilePV(privValKeyFile, privValStateFile) + if err != nil { + return err + } + + logger.Info("Found private validator", "keyFile", privValKeyFile, + "stateFile", privValStateFile) + } else { + pv, err = privval.GenFilePV(privValKeyFile, privValStateFile, keyType) + if err != nil { + return err + } + pv.Save() + logger.Info("Generated private validator", "keyFile", privValKeyFile, + "stateFile", privValStateFile) } - pv.Save() - logger.Info("Generated private validator", "keyFile", privValKeyFile, - "stateFile", privValStateFile) } nodeKeyFile := config.NodeKeyFile() @@ -91,15 +102,18 @@ func initFilesWithConfig(config *cfg.Config) error { ctx, cancel := context.WithTimeout(context.TODO(), ctxTimeout) defer cancel() - pubKey, err := pv.GetPubKey(ctx) - if err != nil { - return fmt.Errorf("can't get pubkey: %w", err) + // if this is a validator we add it to genesis + if pv != nil { + pubKey, err := pv.GetPubKey(ctx) + if err != nil { + return fmt.Errorf("can't get pubkey: %w", err) + } + genDoc.Validators = []types.GenesisValidator{{ + Address: pubKey.Address(), + PubKey: pubKey, + Power: 10, + }} } - genDoc.Validators = []types.GenesisValidator{{ - Address: pubKey.Address(), - PubKey: pubKey, - Power: 10, - }} if err := genDoc.SaveAs(genFile); err != nil { return err @@ -107,5 +121,9 @@ func initFilesWithConfig(config *cfg.Config) error { logger.Info("Generated genesis file", "path", genFile) } + // write config file + cfg.WriteConfigFile(config.RootDir, config) + logger.Info("Generated config", "mode", config.Mode) + return nil } diff --git a/cmd/tendermint/commands/testnet.go b/cmd/tendermint/commands/testnet.go index 6dcf17e1e..5d225413f 100644 --- a/cmd/tendermint/commands/testnet.go +++ b/cmd/tendermint/commands/testnet.go @@ -106,8 +106,7 @@ func testnetFiles(cmd *cobra.Command, args []string) error { } // set mode to validator for testnet - config := cfg.DefaultConfig() - config.Mode = cfg.ModeValidator + config := cfg.DefaultValidatorConfig() // overwrite default config if set and valid if configFile != "" { @@ -242,7 +241,7 @@ func testnetFiles(cmd *cobra.Command, args []string) error { } config.Moniker = moniker(i) - cfg.WriteConfigFile(filepath.Join(nodeDir, "config", "config.toml"), config) + cfg.WriteConfigFile(nodeDir, config) } fmt.Printf("Successfully initialized %v node directories\n", nValidators+nNonValidators) diff --git a/config/config.go b/config/config.go index 8e43e2bb5..79be27d11 100644 --- a/config/config.go +++ b/config/config.go @@ -93,6 +93,13 @@ func DefaultConfig() *Config { } } +// DefaultValidatorConfig returns default config with mode as validator +func DefaultValidatorConfig() *Config { + cfg := DefaultConfig() + cfg.Mode = ModeValidator + return cfg +} + // TestConfig returns a configuration that can be used for testing func TestConfig() *Config { return &Config{ @@ -167,13 +174,13 @@ type BaseConfig struct { //nolint: maligned // A custom human readable name for this node Moniker string `mapstructure:"moniker"` - // Mode of Node: full | validator | seed (default: "full") - // * full (default) - // - all reactors - // - No priv_validator_key.json, priv_validator_state.json + // Mode of Node: full | validator | seed // * validator // - all reactors // - with priv_validator_key.json, priv_validator_state.json + // * full + // - all reactors + // - No priv_validator_key.json, priv_validator_state.json // * seed // - only P2P, PEX Reactor // - No priv_validator_key.json, priv_validator_state.json @@ -346,6 +353,8 @@ func (cfg BaseConfig) ValidateBasic() error { } switch cfg.Mode { case ModeFull, ModeValidator, ModeSeed: + case "": + return errors.New("no mode has been set") default: return fmt.Errorf("unknown mode: %v", cfg.Mode) } diff --git a/config/toml.go b/config/toml.go index f98a35198..84d26935d 100644 --- a/config/toml.go +++ b/config/toml.go @@ -41,32 +41,29 @@ func EnsureRoot(rootDir string) { if err := tmos.EnsureDir(filepath.Join(rootDir, defaultDataDir), DefaultDirPerm); err != nil { panic(err.Error()) } - - configFilePath := filepath.Join(rootDir, defaultConfigFilePath) - - // Write default config file if missing. - if !tmos.FileExists(configFilePath) { - writeDefaultConfigFile(configFilePath) - } -} - -// XXX: this func should probably be called by cmd/tendermint/commands/init.go -// alongside the writing of the genesis.json and priv_validator.json -func writeDefaultConfigFile(configFilePath string) { - WriteConfigFile(configFilePath, DefaultConfig()) } // WriteConfigFile renders config using the template and writes it to configFilePath. -func WriteConfigFile(configFilePath string, config *Config) { +// This function is called by cmd/tendermint/commands/init.go +func WriteConfigFile(rootDir string, config *Config) { var buffer bytes.Buffer if err := configTemplate.Execute(&buffer, config); err != nil { panic(err) } + configFilePath := filepath.Join(rootDir, defaultConfigFilePath) + mustWriteFile(configFilePath, buffer.Bytes(), 0644) } +func writeDefaultConfigFileIfNone(rootDir string) { + configFilePath := filepath.Join(rootDir, defaultConfigFilePath) + if !tmos.FileExists(configFilePath) { + WriteConfigFile(rootDir, DefaultConfig()) + } +} + // Note: any changes to the comments/variables/mapstructure // must be reflected in the appropriate struct in config/config.go const defaultConfigTemplate = `# This is a TOML config file. @@ -88,14 +85,13 @@ proxy-app = "{{ .BaseConfig.ProxyApp }}" # A custom human readable name for this node moniker = "{{ .BaseConfig.Moniker }}" -# Mode of Node: full | validator | seed (default: "full") -# You will need to set it to "validator" if you want to run the node as a validator -# * full node (default) -# - all reactors -# - No priv_validator_key.json, priv_validator_state.json +# Mode of Node: full | validator | seed # * validator node # - all reactors # - with priv_validator_key.json, priv_validator_state.json +# * full node +# - all reactors +# - No priv_validator_key.json, priv_validator_state.json # * seed node # - only P2P, PEX Reactor # - No priv_validator_key.json, priv_validator_state.json @@ -502,15 +498,12 @@ func ResetTestRootWithChainID(testName string, chainID string) *Config { } baseConfig := DefaultBaseConfig() - configFilePath := filepath.Join(rootDir, defaultConfigFilePath) genesisFilePath := filepath.Join(rootDir, baseConfig.Genesis) privKeyFilePath := filepath.Join(rootDir, baseConfig.PrivValidatorKey) privStateFilePath := filepath.Join(rootDir, baseConfig.PrivValidatorState) // Write default config file if missing. - if !tmos.FileExists(configFilePath) { - writeDefaultConfigFile(configFilePath) - } + writeDefaultConfigFileIfNone(rootDir) if !tmos.FileExists(genesisFilePath) { if chainID == "" { chainID = "tendermint_test" diff --git a/config/toml_test.go b/config/toml_test.go index fa587d583..950d218ec 100644 --- a/config/toml_test.go +++ b/config/toml_test.go @@ -30,6 +30,8 @@ func TestEnsureRoot(t *testing.T) { // create root dir EnsureRoot(tmpDir) + WriteConfigFile(tmpDir, DefaultConfig()) + // make sure config is set properly data, err := ioutil.ReadFile(filepath.Join(tmpDir, defaultConfigFilePath)) require.Nil(err) diff --git a/docs/app-dev/getting-started.md b/docs/app-dev/getting-started.md index 126da9371..7d261ae25 100644 --- a/docs/app-dev/getting-started.md +++ b/docs/app-dev/getting-started.md @@ -63,7 +63,7 @@ Tendermint binary installed. If not, follow the steps from before, use: ```sh -tendermint init +tendermint init validator tendermint start ``` diff --git a/docs/architecture/adr-008-priv-validator.md b/docs/architecture/adr-008-priv-validator.md index a8499465c..a3d31048a 100644 --- a/docs/architecture/adr-008-priv-validator.md +++ b/docs/architecture/adr-008-priv-validator.md @@ -4,7 +4,7 @@ Tendermint node's should support only two in-process PrivValidator implementations: - FilePV uses an unencrypted private key in a "priv_validator.json" file - no - configuration required (just `tendermint init`). + configuration required (just `tendermint init validator`). - TCPVal and IPCVal use TCP and Unix sockets respectively to send signing requests to another process - the user is responsible for starting that process themselves. diff --git a/docs/architecture/adr-052-tendermint-mode.md b/docs/architecture/adr-052-tendermint-mode.md index 4848b5356..866f92278 100644 --- a/docs/architecture/adr-052-tendermint-mode.md +++ b/docs/architecture/adr-052-tendermint-mode.md @@ -4,6 +4,7 @@ * 27-11-2019: Initial draft from ADR-051 * 13-01-2020: Separate ADR Tendermint Mode from ADR-051 +* 29-03-2021: Update info regarding defaults ## Context @@ -16,7 +17,7 @@ We would like to suggest a simple Tendermint mode abstraction. These modes will live under one binary, and when initializing a node the user will be able to specify which node they would like to create. - Which reactor, component to include for each node - - full *(default)* + - full - switch, transport - reactors - mempool @@ -46,7 +47,8 @@ We would like to suggest a simple Tendermint mode abstraction. These modes will - We would like to suggest by introducing `mode` parameter in `config.toml` and cli - `mode = "{{ .BaseConfig.Mode }}"` in `config.toml` - `tendermint start --mode validator` in cli - - full | validator | seednode (default: "full") + - full | validator | seednode + - There will be no default. Users will need to specify when they run `tendermint init` - RPC modification - `host:26657/status` - return empty `validator_info` when in full mode diff --git a/docs/introduction/install.md b/docs/introduction/install.md index 4d72cb90c..42394b9d6 100644 --- a/docs/introduction/install.md +++ b/docs/introduction/install.md @@ -56,7 +56,7 @@ tendermint version To start a one-node blockchain with a simple in-process application: ```sh -tendermint init +tendermint init validator tendermint start --proxy-app=kvstore ``` diff --git a/docs/introduction/quick-start.md b/docs/introduction/quick-start.md index c1a81c075..bc6f36af0 100644 --- a/docs/introduction/quick-start.md +++ b/docs/introduction/quick-start.md @@ -34,7 +34,7 @@ For manual installation, see the [install instructions](install.md) Running: ```sh -tendermint init +tendermint init validator ``` will create the required files for a single, local node. diff --git a/docs/nodes/configuration.md b/docs/nodes/configuration.md index eaf448c7e..35e639754 100644 --- a/docs/nodes/configuration.md +++ b/docs/nodes/configuration.md @@ -41,18 +41,17 @@ moniker = "anonymous" # and verifying their commits fast-sync = true -# Mode of Node: full | validator | seed -# You will need to set it to "validator" if you want to run the node as a validator -# * full node (default) -# - all reactors -# - No priv_validator_key.json, priv_validator_state.json -# * validator node +# Mode of Node: full | validator | seed (default: "validator") +# * validator node (default) # - all reactors # - with priv_validator_key.json, priv_validator_state.json +# * full node +# - all reactors +# - No priv_validator_key.json, priv_validator_state.json # * seed node # - only P2P, PEX Reactor # - No priv_validator_key.json, priv_validator_state.json -mode = "full" +mode = "validator" # Database backend: goleveldb | cleveldb | boltdb | rocksdb | badgerdb # * goleveldb (github.com/syndtr/goleveldb - most popular implementation) diff --git a/docs/tendermint-core/using-tendermint.md b/docs/tendermint-core/using-tendermint.md index f743a5dec..e6204c89e 100644 --- a/docs/tendermint-core/using-tendermint.md +++ b/docs/tendermint-core/using-tendermint.md @@ -21,7 +21,7 @@ this by setting the `TMHOME` environment variable. Initialize the root directory by running: ```sh -tendermint init +tendermint init validator ``` This will create a new private key (`priv_validator_key.json`), and a diff --git a/docs/tools/remote-signer-validation.md b/docs/tools/remote-signer-validation.md index 3099d7e4d..80a6a64bc 100644 --- a/docs/tools/remote-signer-validation.md +++ b/docs/tools/remote-signer-validation.md @@ -75,7 +75,7 @@ example, we will simply export a signing key from our local Tendermint instance. # Will generate all necessary Tendermint configuration files, including: # - ~/.tendermint/config/priv_validator_key.json # - ~/.tendermint/data/priv_validator_state.json -tendermint init +tendermint init validator # Extract the signing key from our local Tendermint instance tm-signer-harness extract_key \ # Use the "extract_key" command diff --git a/docs/tutorials/go-built-in.md b/docs/tutorials/go-built-in.md index f32a17eb7..040891637 100644 --- a/docs/tutorials/go-built-in.md +++ b/docs/tutorials/go-built-in.md @@ -393,7 +393,7 @@ func main() { func newTendermint(app abci.Application, configFile string) (*nm.Node, error) { // read config - config := cfg.DefaultConfig() + config := cfg.DefaultValidatorConfig() config.RootDir = filepath.Dir(filepath.Dir(configFile)) viper.SetConfigFile(configFile) if err := viper.ReadInConfig(); err != nil { @@ -503,7 +503,7 @@ of one communicating through a socket or gRPC. which we will generate later using the `tendermint init` command. ```go -config := cfg.DefaultConfig() +config := cfg.DefaultValidatorConfig() config.RootDir = filepath.Dir(filepath.Dir(configFile)) viper.SetConfigFile(configFile) if err := viper.ReadInConfig(); err != nil { @@ -604,7 +604,7 @@ go build ``` To create a default configuration, nodeKey and private validator files, let's -execute `tendermint init`. But before we do that, we will need to install +execute `tendermint init validator`. But before we do that, we will need to install Tendermint Core. Please refer to [the official guide](https://docs.tendermint.com/master/introduction/install.html). If you're installing from source, don't forget to checkout the latest release (`git @@ -613,11 +613,12 @@ major version. ```bash $ rm -rf /tmp/example -$ TMHOME="/tmp/example" tendermint init +$ TMHOME="/tmp/example" tendermint init validator I[2019-07-16|18:40:36.480] Generated private validator module=main keyFile=/tmp/example/config/priv_validator_key.json stateFile=/tmp/example2/data/priv_validator_state.json I[2019-07-16|18:40:36.481] Generated node key module=main path=/tmp/example/config/node_key.json I[2019-07-16|18:40:36.482] Generated genesis file module=main path=/tmp/example/config/genesis.json +I[2019-07-16|18:40:36.483] Generated config module=main mode=validator ``` We are ready to start our application: diff --git a/docs/tutorials/go.md b/docs/tutorials/go.md index a01292112..02625291f 100644 --- a/docs/tutorials/go.md +++ b/docs/tutorials/go.md @@ -461,7 +461,7 @@ go build ``` To create a default configuration, nodeKey and private validator files, let's -execute `tendermint init`. But before we do that, we will need to install +execute `tendermint init validator`. But before we do that, we will need to install Tendermint Core. Please refer to [the official guide](https://docs.tendermint.com/master/introduction/install.html). If you're installing from source, don't forget to checkout the latest release (`git @@ -470,11 +470,12 @@ major version. ```bash rm -rf /tmp/example -TMHOME="/tmp/example" tendermint init +TMHOME="/tmp/example" tendermint init validator I[2019-07-16|18:20:36.480] Generated private validator module=main keyFile=/tmp/example/config/priv_validator_key.json stateFile=/tmp/example2/data/priv_validator_state.json I[2019-07-16|18:20:36.481] Generated node key module=main path=/tmp/example/config/node_key.json I[2019-07-16|18:20:36.482] Generated genesis file module=main path=/tmp/example/config/genesis.json +I[2019-07-16|18:20:36.483] Generated config module=main mode=validator ``` Feel free to explore the generated files, which can be found at diff --git a/docs/tutorials/java.md b/docs/tutorials/java.md index f27ba1a9f..c36cafcf0 100644 --- a/docs/tutorials/java.md +++ b/docs/tutorials/java.md @@ -550,11 +550,12 @@ Tendermint Core. $ rm -rf /tmp/example $ cd $GOPATH/src/github.com/tendermint/tendermint $ make install -$ TMHOME="/tmp/example" tendermint init +$ TMHOME="/tmp/example" tendermint init validator I[2019-07-16|18:20:36.480] Generated private validator module=main keyFile=/tmp/example/config/priv_validator_key.json stateFile=/tmp/example2/data/priv_validator_state.json I[2019-07-16|18:20:36.481] Generated node key module=main path=/tmp/example/config/node_key.json I[2019-07-16|18:20:36.482] Generated genesis file module=main path=/tmp/example/config/genesis.json +I[2019-07-16|18:20:36.483] Generated config module=main mode=validator ``` Feel free to explore the generated files, which can be found at diff --git a/docs/tutorials/kotlin.md b/docs/tutorials/kotlin.md index a46053427..6fa1d1894 100644 --- a/docs/tutorials/kotlin.md +++ b/docs/tutorials/kotlin.md @@ -517,18 +517,19 @@ class GrpcServer( ## 1.5 Getting Up and Running To create a default configuration, nodeKey and private validator files, let's -execute `tendermint init`. But before we do that, we will need to install +execute `tendermint init validator`. But before we do that, we will need to install Tendermint Core. ```bash rm -rf /tmp/example cd $GOPATH/src/github.com/tendermint/tendermint make install -TMHOME="/tmp/example" tendermint init +TMHOME="/tmp/example" tendermint init validator I[2019-07-16|18:20:36.480] Generated private validator module=main keyFile=/tmp/example/config/priv_validator_key.json stateFile=/tmp/example2/data/priv_validator_state.json I[2019-07-16|18:20:36.481] Generated node key module=main path=/tmp/example/config/node_key.json I[2019-07-16|18:20:36.482] Generated genesis file module=main path=/tmp/example/config/genesis.json +I[2019-07-16|18:20:36.482] Generated config module=main mode=validator ``` Feel free to explore the generated files, which can be found at diff --git a/test/app/test.sh b/test/app/test.sh index d415bc10e..67fb2d9b6 100755 --- a/test/app/test.sh +++ b/test/app/test.sh @@ -13,7 +13,7 @@ export TMHOME=$HOME/.tendermint_app function kvstore_over_socket(){ rm -rf $TMHOME - tendermint init + tendermint init validator echo "Starting kvstore_over_socket" abci-cli kvstore > /dev/null & pid_kvstore=$! @@ -30,7 +30,7 @@ function kvstore_over_socket(){ # start tendermint first function kvstore_over_socket_reorder(){ rm -rf $TMHOME - tendermint init + tendermint init validator echo "Starting kvstore_over_socket_reorder (ie. start tendermint first)" tendermint start --mode validator > tendermint.log & pid_tendermint=$! @@ -48,7 +48,7 @@ function kvstore_over_socket_reorder(){ function counter_over_socket() { rm -rf $TMHOME - tendermint init + tendermint init validator echo "Starting counter_over_socket" abci-cli counter --serial > /dev/null & pid_counter=$! @@ -64,7 +64,7 @@ function counter_over_socket() { function counter_over_grpc() { rm -rf $TMHOME - tendermint init + tendermint init validator echo "Starting counter_over_grpc" abci-cli counter --serial --abci grpc > /dev/null & pid_counter=$! @@ -80,7 +80,7 @@ function counter_over_grpc() { function counter_over_grpc_grpc() { rm -rf $TMHOME - tendermint init + tendermint init validator echo "Starting counter_over_grpc_grpc (ie. with grpc broadcast_tx)" abci-cli counter --serial --abci grpc > /dev/null & pid_counter=$! diff --git a/test/e2e/runner/setup.go b/test/e2e/runner/setup.go index 22ff58b15..b2dbfe5a9 100644 --- a/test/e2e/runner/setup.go +++ b/test/e2e/runner/setup.go @@ -85,7 +85,7 @@ func Setup(testnet *e2e.Testnet) error { if err != nil { return err } - config.WriteConfigFile(filepath.Join(nodeDir, "config", "config.toml"), cfg) // panics + config.WriteConfigFile(nodeDir, cfg) // panics appCfg, err := MakeAppConfig(node) if err != nil { From 053651160f496bb44b107a434e3e6482530bb287 Mon Sep 17 00:00:00 2001 From: Lanie Hei Date: Fri, 2 Apr 2021 13:25:24 -0500 Subject: [PATCH 19/24] Adds missing line break (#6309) Adds small fix to the docs. Line 67 specifies "two key changes". I think this is where the second bullet point was meant to start. --- docs/introduction/what-is-tendermint.md | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/docs/introduction/what-is-tendermint.md b/docs/introduction/what-is-tendermint.md index 9cff48a2a..2386626ea 100644 --- a/docs/introduction/what-is-tendermint.md +++ b/docs/introduction/what-is-tendermint.md @@ -68,11 +68,11 @@ Tendermint is in essence similar software, but with two key differences: - It is Byzantine Fault Tolerant, meaning it can only tolerate up to a 1/3 of failures, but those failures can include arbitrary behaviour - - including hacking and malicious attacks. - It does not specify a - particular application, like a fancy key-value store. Instead, it - focuses on arbitrary state machine replication, so developers can build - the application logic that's right for them, from key-value store to - cryptocurrency to e-voting platform and beyond. + including hacking and malicious attacks. +- It does not specify a particular application, like a fancy key-value + store. Instead, it focuses on arbitrary state machine replication, + so developers can build the application logic that's right for them, + from key-value store to cryptocurrency to e-voting platform and beyond. ### Bitcoin, Ethereum, etc From 46e06c97320bc61c4d98d3018f59d47ec69863c9 Mon Sep 17 00:00:00 2001 From: Aleksandr Bezobchuk Date: Sat, 3 Apr 2021 12:18:54 -0400 Subject: [PATCH 20/24] state: fix block event indexing reserved key check (#6314) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit copy 🍝 bug when porting similar logic from the tx indexing code. --- state/indexer/block/kv/kv.go | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/state/indexer/block/kv/kv.go b/state/indexer/block/kv/kv.go index 916c5f8ae..901f0e1d2 100644 --- a/state/indexer/block/kv/kv.go +++ b/state/indexer/block/kv/kv.go @@ -469,9 +469,10 @@ func (idx *BlockerIndexer) indexEvents(batch dbm.Batch, events []abci.Event, typ // index iff the event specified index:true and it's not a reserved event compositeKey := fmt.Sprintf("%s.%s", event.Type, string(attr.Key)) - if compositeKey == types.TxHashKey || compositeKey == types.TxHeightKey { + if compositeKey == types.BlockHeightKey { return fmt.Errorf("event type and attribute key \"%s\" is reserved; please use a different key", compositeKey) } + if attr.GetIndex() { key, err := eventKey(compositeKey, typ, string(attr.Value), height) if err != nil { From 1c4dbe30d40bb4c1dc59a2247d5113b78702a014 Mon Sep 17 00:00:00 2001 From: Sam Kleinman Date: Sat, 3 Apr 2021 14:25:15 -0400 Subject: [PATCH 21/24] abci: change client to use multi-reader mutexes (#6306) --- abci/client/client.go | 8 ++++---- abci/client/grpc_client.go | 8 ++++---- abci/client/local_client.go | 26 ++++++++++++++------------ abci/client/socket_client.go | 8 ++++---- consensus/byzantine_test.go | 2 +- consensus/common_test.go | 2 +- consensus/reactor_test.go | 2 +- proxy/client.go | 4 ++-- 8 files changed, 31 insertions(+), 29 deletions(-) diff --git a/abci/client/client.go b/abci/client/client.go index 4cc5dafff..bcba3bec3 100644 --- a/abci/client/client.go +++ b/abci/client/client.go @@ -87,7 +87,7 @@ type ReqRes struct { *sync.WaitGroup *types.Response // Not set atomically, so be sure to use WaitGroup. - mtx tmsync.Mutex + mtx tmsync.RWMutex done bool // Gets set to true once *after* WaitGroup.Done(). cb func(*types.Response) // A single callback that may be set. } @@ -137,16 +137,16 @@ func (r *ReqRes) InvokeCallback() { // // ref: https://github.com/tendermint/tendermint/issues/5439 func (r *ReqRes) GetCallback() func(*types.Response) { - r.mtx.Lock() - defer r.mtx.Unlock() + r.mtx.RLock() + defer r.mtx.RUnlock() return r.cb } // SetDone marks the ReqRes object as done. func (r *ReqRes) SetDone() { r.mtx.Lock() + defer r.mtx.Unlock() r.done = true - r.mtx.Unlock() } func waitGroup1() (wg *sync.WaitGroup) { diff --git a/abci/client/grpc_client.go b/abci/client/grpc_client.go index b10eebd9e..18d9cb168 100644 --- a/abci/client/grpc_client.go +++ b/abci/client/grpc_client.go @@ -24,7 +24,7 @@ type grpcClient struct { conn *grpc.ClientConn chReqRes chan *ReqRes // dispatches "async" responses to callbacks *in order*, needed by mempool - mtx tmsync.Mutex + mtx tmsync.RWMutex addr string err error resCb func(*types.Request, *types.Response) // listens to all callbacks @@ -149,8 +149,8 @@ func (cli *grpcClient) StopForError(err error) { } func (cli *grpcClient) Error() error { - cli.mtx.Lock() - defer cli.mtx.Unlock() + cli.mtx.RLock() + defer cli.mtx.RUnlock() return cli.err } @@ -158,8 +158,8 @@ func (cli *grpcClient) Error() error { // NOTE: callback may get internally generated flush responses. func (cli *grpcClient) SetResponseCallback(resCb Callback) { cli.mtx.Lock() + defer cli.mtx.Unlock() cli.resCb = resCb - cli.mtx.Unlock() } //---------------------------------------- diff --git a/abci/client/local_client.go b/abci/client/local_client.go index 92ca0804f..d25a98b4d 100644 --- a/abci/client/local_client.go +++ b/abci/client/local_client.go @@ -15,7 +15,7 @@ import ( type localClient struct { service.BaseService - mtx *tmsync.Mutex + mtx *tmsync.RWMutex types.Application Callback } @@ -26,22 +26,24 @@ var _ Client = (*localClient)(nil) // methods of the given app. // // Both Async and Sync methods ignore the given context.Context parameter. -func NewLocalClient(mtx *tmsync.Mutex, app types.Application) Client { +func NewLocalClient(mtx *tmsync.RWMutex, app types.Application) Client { if mtx == nil { - mtx = new(tmsync.Mutex) + mtx = &tmsync.RWMutex{} } + cli := &localClient{ mtx: mtx, Application: app, } + cli.BaseService = *service.NewBaseService(nil, "localClient", cli) return cli } func (app *localClient) SetResponseCallback(cb Callback) { app.mtx.Lock() + defer app.mtx.Unlock() app.Callback = cb - app.mtx.Unlock() } // TODO: change types.Application to include Error()? @@ -65,8 +67,8 @@ func (app *localClient) EchoAsync(ctx context.Context, msg string) (*ReqRes, err } func (app *localClient) InfoAsync(ctx context.Context, req types.RequestInfo) (*ReqRes, error) { - app.mtx.Lock() - defer app.mtx.Unlock() + app.mtx.RLock() + defer app.mtx.RUnlock() res := app.Application.Info(req) return app.callback( @@ -98,8 +100,8 @@ func (app *localClient) CheckTxAsync(ctx context.Context, req types.RequestCheck } func (app *localClient) QueryAsync(ctx context.Context, req types.RequestQuery) (*ReqRes, error) { - app.mtx.Lock() - defer app.mtx.Unlock() + app.mtx.RLock() + defer app.mtx.RUnlock() res := app.Application.Query(req) return app.callback( @@ -213,8 +215,8 @@ func (app *localClient) EchoSync(ctx context.Context, msg string) (*types.Respon } func (app *localClient) InfoSync(ctx context.Context, req types.RequestInfo) (*types.ResponseInfo, error) { - app.mtx.Lock() - defer app.mtx.Unlock() + app.mtx.RLock() + defer app.mtx.RUnlock() res := app.Application.Info(req) return &res, nil @@ -247,8 +249,8 @@ func (app *localClient) QuerySync( ctx context.Context, req types.RequestQuery, ) (*types.ResponseQuery, error) { - app.mtx.Lock() - defer app.mtx.Unlock() + app.mtx.RLock() + defer app.mtx.RUnlock() res := app.Application.Query(req) return &res, nil diff --git a/abci/client/socket_client.go b/abci/client/socket_client.go index 0569f4c6b..3e4034902 100644 --- a/abci/client/socket_client.go +++ b/abci/client/socket_client.go @@ -43,7 +43,7 @@ type socketClient struct { reqQueue chan *reqResWithContext flushTimer *timer.ThrottleTimer - mtx tmsync.Mutex + mtx tmsync.RWMutex err error reqSent *list.List // list of requests sent, waiting for response resCb func(*types.Request, *types.Response) // called on all requests, if set. @@ -108,8 +108,8 @@ func (cli *socketClient) OnStop() { // Error returns an error if the client was stopped abruptly. func (cli *socketClient) Error() error { - cli.mtx.Lock() - defer cli.mtx.Unlock() + cli.mtx.RLock() + defer cli.mtx.RUnlock() return cli.err } @@ -119,8 +119,8 @@ func (cli *socketClient) Error() error { // NOTE: callback may get internally generated flush responses. func (cli *socketClient) SetResponseCallback(resCb Callback) { cli.mtx.Lock() + defer cli.mtx.Unlock() cli.resCb = resCb - cli.mtx.Unlock() } //---------------------------------------- diff --git a/consensus/byzantine_test.go b/consensus/byzantine_test.go index f0b65ee57..46b752d32 100644 --- a/consensus/byzantine_test.go +++ b/consensus/byzantine_test.go @@ -58,7 +58,7 @@ func TestByzantinePrevoteEquivocation(t *testing.T) { blockStore := store.NewBlockStore(blockDB) // one for mempool, one for consensus - mtx := new(tmsync.Mutex) + mtx := new(tmsync.RWMutex) proxyAppConnMem := abcicli.NewLocalClient(mtx, app) proxyAppConnCon := abcicli.NewLocalClient(mtx, app) diff --git a/consensus/common_test.go b/consensus/common_test.go index bd844982c..953339c20 100644 --- a/consensus/common_test.go +++ b/consensus/common_test.go @@ -392,7 +392,7 @@ func newStateWithConfigAndBlockStore( blockStore := store.NewBlockStore(blockDB) // one for mempool, one for consensus - mtx := new(tmsync.Mutex) + mtx := new(tmsync.RWMutex) proxyAppConnMem := abcicli.NewLocalClient(mtx, app) proxyAppConnCon := abcicli.NewLocalClient(mtx, app) diff --git a/consensus/reactor_test.go b/consensus/reactor_test.go index a53561cff..3d14533c5 100644 --- a/consensus/reactor_test.go +++ b/consensus/reactor_test.go @@ -303,7 +303,7 @@ func TestReactorWithEvidence(t *testing.T) { blockStore := store.NewBlockStore(blockDB) // one for mempool, one for consensus - mtx := new(tmsync.Mutex) + mtx := new(tmsync.RWMutex) proxyAppConnMem := abcicli.NewLocalClient(mtx, app) proxyAppConnCon := abcicli.NewLocalClient(mtx, app) diff --git a/proxy/client.go b/proxy/client.go index 1dc6d8853..27baa9738 100644 --- a/proxy/client.go +++ b/proxy/client.go @@ -20,7 +20,7 @@ type ClientCreator interface { // local proxy uses a mutex on an in-proc app type localClientCreator struct { - mtx *tmsync.Mutex + mtx *tmsync.RWMutex app types.Application } @@ -28,7 +28,7 @@ type localClientCreator struct { // which will be running locally. func NewLocalClientCreator(app types.Application) ClientCreator { return &localClientCreator{ - mtx: new(tmsync.Mutex), + mtx: new(tmsync.RWMutex), app: app, } } From bcdf923cb88eb48889e4d1d0f9763842c85ad436 Mon Sep 17 00:00:00 2001 From: Sam Kleinman Date: Mon, 5 Apr 2021 10:54:32 -0400 Subject: [PATCH 22/24] p2p: improve router test stability (#6310) --- p2p/router_test.go | 26 ++++++++++++++++++++++++++ 1 file changed, 26 insertions(+) diff --git a/p2p/router_test.go b/p2p/router_test.go index 748b4de2e..acc8fca05 100644 --- a/p2p/router_test.go +++ b/p2p/router_test.go @@ -97,6 +97,8 @@ func TestRouter_Channel(t *testing.T) { // Set up a router with no transports (so no peers). peerManager, err := p2p.NewPeerManager(selfID, dbm.NewMemDB(), p2p.PeerManagerOptions{}) require.NoError(t, err) + defer peerManager.Close() + router, err := p2p.NewRouter( log.TestingLogger(), p2p.NopMetrics(), @@ -336,6 +338,8 @@ func TestRouter_AcceptPeers(t *testing.T) { mockConnection.On("Close").Run(func(_ mock.Arguments) { closer.Close() }).Return(nil) mockConnection.On("RemoteEndpoint").Return(p2p.Endpoint{}) if tc.ok { + // without the sleep after RequireUpdate this method isn't + // always called. Consider making this call optional. mockConnection.On("ReceiveMessage").Return(chID, nil, io.EOF) } @@ -349,6 +353,8 @@ func TestRouter_AcceptPeers(t *testing.T) { // Set up and start the router. peerManager, err := p2p.NewPeerManager(selfID, dbm.NewMemDB(), p2p.PeerManagerOptions{}) require.NoError(t, err) + defer peerManager.Close() + sub := peerManager.Subscribe() defer sub.Close() @@ -369,6 +375,9 @@ func TestRouter_AcceptPeers(t *testing.T) { NodeID: tc.peerInfo.NodeID, Status: p2p.PeerStatusUp, }) + // force a context switch so that the + // connection is handled. + time.Sleep(time.Millisecond) sub.Close() } else { select { @@ -399,6 +408,8 @@ func TestRouter_AcceptPeers_Error(t *testing.T) { // Set up and start the router. peerManager, err := p2p.NewPeerManager(selfID, dbm.NewMemDB(), p2p.PeerManagerOptions{}) require.NoError(t, err) + defer peerManager.Close() + router, err := p2p.NewRouter( log.TestingLogger(), p2p.NopMetrics(), @@ -431,6 +442,8 @@ func TestRouter_AcceptPeers_ErrorEOF(t *testing.T) { // Set up and start the router. peerManager, err := p2p.NewPeerManager(selfID, dbm.NewMemDB(), p2p.PeerManagerOptions{}) require.NoError(t, err) + defer peerManager.Close() + router, err := p2p.NewRouter( log.TestingLogger(), p2p.NopMetrics(), @@ -477,6 +490,8 @@ func TestRouter_AcceptPeers_HeadOfLineBlocking(t *testing.T) { // Set up and start the router. peerManager, err := p2p.NewPeerManager(selfID, dbm.NewMemDB(), p2p.PeerManagerOptions{}) require.NoError(t, err) + defer peerManager.Close() + router, err := p2p.NewRouter( log.TestingLogger(), p2p.NopMetrics(), @@ -532,6 +547,8 @@ func TestRouter_DialPeers(t *testing.T) { mockConnection.On("Close").Run(func(_ mock.Arguments) { closer.Close() }).Return(nil) } if tc.ok { + // without the sleep after RequireUpdate this method isn't + // always called. Consider making this call optional. mockConnection.On("ReceiveMessage").Return(chID, nil, io.EOF) } @@ -554,6 +571,8 @@ func TestRouter_DialPeers(t *testing.T) { // Set up and start the router. peerManager, err := p2p.NewPeerManager(selfID, dbm.NewMemDB(), p2p.PeerManagerOptions{}) require.NoError(t, err) + defer peerManager.Close() + require.NoError(t, peerManager.Add(address)) sub := peerManager.Subscribe() defer sub.Close() @@ -575,6 +594,9 @@ func TestRouter_DialPeers(t *testing.T) { NodeID: tc.peerInfo.NodeID, Status: p2p.PeerStatusUp, }) + // force a context switch so that the + // connection is handled. + time.Sleep(time.Millisecond) sub.Close() } else { select { @@ -624,6 +646,8 @@ func TestRouter_DialPeers_Parallel(t *testing.T) { // Set up and start the router. peerManager, err := p2p.NewPeerManager(selfID, dbm.NewMemDB(), p2p.PeerManagerOptions{}) require.NoError(t, err) + defer peerManager.Close() + require.NoError(t, peerManager.Add(a)) require.NoError(t, peerManager.Add(b)) require.NoError(t, peerManager.Add(c)) @@ -680,6 +704,8 @@ func TestRouter_EvictPeers(t *testing.T) { // Set up and start the router. peerManager, err := p2p.NewPeerManager(selfID, dbm.NewMemDB(), p2p.PeerManagerOptions{}) require.NoError(t, err) + defer peerManager.Close() + sub := peerManager.Subscribe() defer sub.Close() From 6d9372bd390bf1c0ff9b9e72c002a17129862379 Mon Sep 17 00:00:00 2001 From: Sam Kleinman Date: Mon, 5 Apr 2021 12:39:04 -0400 Subject: [PATCH 23/24] test: improve cleanup for data and disk use (#6311) --- consensus/common_test.go | 12 ++++++++++++ consensus/replay_file.go | 2 +- node/node.go | 4 +++- node/node_test.go | 4 +++- proxy/client.go | 23 ++++++++++++++++------- rpc/client/main_test.go | 1 + 6 files changed, 36 insertions(+), 10 deletions(-) diff --git a/consensus/common_test.go b/consensus/common_test.go index 953339c20..48a452de1 100644 --- a/consensus/common_test.go +++ b/consensus/common_test.go @@ -4,6 +4,7 @@ import ( "bytes" "context" "fmt" + "io" "io/ioutil" "os" "path/filepath" @@ -704,7 +705,10 @@ func randConsensusState( css := make([]*State, nValidators) logger := consensusLogger() + closeFuncs := make([]func() error, 0, nValidators) + configRootDirs := make([]string, 0, nValidators) + for i := 0; i < nValidators; i++ { stateDB := dbm.NewMemDB() // each state needs its own db stateStore := sm.NewStore(stateDB) @@ -719,6 +723,11 @@ func randConsensusState( ensureDir(filepath.Dir(thisConfig.Consensus.WalFile()), 0700) // dir for wal app := appFunc() + + if appCloser, ok := app.(io.Closer); ok { + closeFuncs = append(closeFuncs, appCloser.Close) + } + vals := types.TM2PB.ValidatorUpdates(state.Validators) app.InitChain(abci.RequestInitChain{Validators: vals}) @@ -728,6 +737,9 @@ func randConsensusState( } return css, func() { + for _, closer := range closeFuncs { + _ = closer() + } for _, dir := range configRootDirs { os.RemoveAll(dir) } diff --git a/consensus/replay_file.go b/consensus/replay_file.go index 4bf7466ab..2244d868e 100644 --- a/consensus/replay_file.go +++ b/consensus/replay_file.go @@ -308,7 +308,7 @@ func newConsensusStateForReplay(config cfg.BaseConfig, csConfig *cfg.ConsensusCo } // Create proxyAppConn connection (consensus, mempool, query) - clientCreator := proxy.DefaultClientCreator(config.ProxyApp, config.ABCI, config.DBDir()) + clientCreator, _ := proxy.DefaultClientCreator(config.ProxyApp, config.ABCI, config.DBDir()) proxyApp := proxy.NewAppConns(clientCreator) err = proxyApp.Start() if err != nil { diff --git a/node/node.go b/node/node.go index 4b2b584ae..477b4ca9a 100644 --- a/node/node.go +++ b/node/node.go @@ -126,10 +126,12 @@ func DefaultNewNode(config *cfg.Config, logger log.Logger) (*Node, error) { } else { pval = nil } + + appClient, _ := proxy.DefaultClientCreator(config.ProxyApp, config.ABCI, config.DBDir()) return NewNode(config, pval, nodeKey, - proxy.DefaultClientCreator(config.ProxyApp, config.ABCI, config.DBDir()), + appClient, DefaultGenesisDocProviderFunc(config), DefaultDBProvider, DefaultMetricsProvider(config.Instrumentation), diff --git a/node/node_test.go b/node/node_test.go index d55a8d280..3290dbcd2 100644 --- a/node/node_test.go +++ b/node/node_test.go @@ -499,10 +499,12 @@ func TestNodeNewNodeCustomReactors(t *testing.T) { pval, err := privval.LoadOrGenFilePV(config.PrivValidatorKeyFile(), config.PrivValidatorStateFile()) require.NoError(t, err) + appClient, closer := proxy.DefaultClientCreator(config.ProxyApp, config.ABCI, config.DBDir()) + t.Cleanup(func() { closer.Close() }) n, err := NewNode(config, pval, nodeKey, - proxy.DefaultClientCreator(config.ProxyApp, config.ABCI, config.DBDir()), + appClient, DefaultGenesisDocProviderFunc(config), DefaultDBProvider, DefaultMetricsProvider(config.Instrumentation), diff --git a/proxy/client.go b/proxy/client.go index 27baa9738..f57c603ca 100644 --- a/proxy/client.go +++ b/proxy/client.go @@ -2,6 +2,7 @@ package proxy import ( "fmt" + "io" abcicli "github.com/tendermint/tendermint/abci/client" "github.com/tendermint/tendermint/abci/example/counter" @@ -69,20 +70,28 @@ func (r *remoteClientCreator) NewABCIClient() (abcicli.Client, error) { // DefaultClientCreator returns a default ClientCreator, which will create a // local client if addr is one of: 'counter', 'counter_serial', 'kvstore', // 'persistent_kvstore' or 'noop', otherwise - a remote client. -func DefaultClientCreator(addr, transport, dbDir string) ClientCreator { +// +// The Closer is a noop except for persistent_kvstore applications, +// which will clean up the store. +func DefaultClientCreator(addr, transport, dbDir string) (ClientCreator, io.Closer) { switch addr { case "counter": - return NewLocalClientCreator(counter.NewApplication(false)) + return NewLocalClientCreator(counter.NewApplication(false)), noopCloser{} case "counter_serial": - return NewLocalClientCreator(counter.NewApplication(true)) + return NewLocalClientCreator(counter.NewApplication(true)), noopCloser{} case "kvstore": - return NewLocalClientCreator(kvstore.NewApplication()) + return NewLocalClientCreator(kvstore.NewApplication()), noopCloser{} case "persistent_kvstore": - return NewLocalClientCreator(kvstore.NewPersistentKVStoreApplication(dbDir)) + app := kvstore.NewPersistentKVStoreApplication(dbDir) + return NewLocalClientCreator(app), app case "noop": - return NewLocalClientCreator(types.NewBaseApplication()) + return NewLocalClientCreator(types.NewBaseApplication()), noopCloser{} default: mustConnect := false // loop retrying - return NewRemoteClientCreator(addr, transport, mustConnect) + return NewRemoteClientCreator(addr, transport, mustConnect), noopCloser{} } } + +type noopCloser struct{} + +func (noopCloser) Close() error { return nil } diff --git a/rpc/client/main_test.go b/rpc/client/main_test.go index c97311c81..cab8b7cdd 100644 --- a/rpc/client/main_test.go +++ b/rpc/client/main_test.go @@ -26,6 +26,7 @@ func TestMain(m *testing.M) { // and shut down proper at the end rpctest.StopTendermint(node) + app.Close() _ = os.RemoveAll(dir) os.Exit(code) } From 10f4b7de64d767781ed4c2d11aeb932c3fc3ded4 Mon Sep 17 00:00:00 2001 From: "dependabot[bot]" <49699333+dependabot[bot]@users.noreply.github.com> Date: Mon, 5 Apr 2021 12:47:27 -0400 Subject: [PATCH 24/24] build(deps): Bump golangci/golangci-lint-action from v2.5.1 to v2.5.2 (#6317) --- .github/workflows/lint.yaml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/workflows/lint.yaml b/.github/workflows/lint.yaml index 8e4fcbdad..dc4263423 100644 --- a/.github/workflows/lint.yaml +++ b/.github/workflows/lint.yaml @@ -20,7 +20,7 @@ jobs: **/**.go go.mod go.sum - - uses: golangci/golangci-lint-action@v2.5.1 + - uses: golangci/golangci-lint-action@v2.5.2 with: # Required: the version of golangci-lint is required and must be specified without patch version: we always use the latest patch version. version: v1.38