[](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)
---
.github/workflows/docker.yml | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/.github/workflows/docker.yml b/.github/workflows/docker.yml
index 524df1ef8..1c16133e0 100644
--- a/.github/workflows/docker.yml
+++ b/.github/workflows/docker.yml
@@ -39,7 +39,7 @@ jobs:
platforms: all
- name: Set up Docker Build
- uses: docker/setup-buildx-action@v1.7.0
+ uses: docker/setup-buildx-action@v2.0.0
- name: Login to DockerHub
if: ${{ github.event_name != 'pull_request' }}
From 083716b22a8d02b332950655634dd5bc9a7afd71 Mon Sep 17 00:00:00 2001
From: "dependabot[bot]" <49699333+dependabot[bot]@users.noreply.github.com>
Date: Mon, 9 May 2022 14:33:45 +0000
Subject: [PATCH 02/39] build(deps): Bump docker/build-push-action from 2.10.0
to 3.0.0 (#8482)
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit
Bumps [docker/build-push-action](https://github.com/docker/build-push-action) from 2.10.0 to 3.0.0.
Release notes
[](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)
---
.github/workflows/docker.yml | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/.github/workflows/docker.yml b/.github/workflows/docker.yml
index 1c16133e0..7547e8591 100644
--- a/.github/workflows/docker.yml
+++ b/.github/workflows/docker.yml
@@ -49,7 +49,7 @@ jobs:
password: ${{ secrets.DOCKERHUB_TOKEN }}
- name: Publish to Docker Hub
- uses: docker/build-push-action@v2.10.0
+ uses: docker/build-push-action@v3.0.0
with:
context: .
file: ./DOCKER/Dockerfile
From b52b8f2740bf075e2a917784227a0c4ff4b9f470 Mon Sep 17 00:00:00 2001
From: "dependabot[bot]" <49699333+dependabot[bot]@users.noreply.github.com>
Date: Mon, 9 May 2022 14:35:43 +0000
Subject: [PATCH 03/39] build(deps): Bump docker/login-action from 1.14.1 to
2.0.0 (#8481)
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit
Bumps [docker/login-action](https://github.com/docker/login-action) from 1.14.1 to 2.0.0.
Release notes
[](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 aea70543b..9af23abaa 100644
--- a/go.mod
+++ b/go.mod
@@ -174,7 +174,7 @@ require (
github.com/pmezard/go-difflib v1.0.0 // indirect
github.com/polyfloyd/go-errorlint v0.0.0-20211125173453-6d6d39c5bb8b // indirect
github.com/prometheus/client_model v0.2.0
- github.com/prometheus/common v0.32.1
+ github.com/prometheus/common v0.34.0
github.com/prometheus/procfs v0.7.3 // indirect
github.com/quasilyte/go-ruleguard v0.3.16-0.20220213074421-6aa060fab41a // indirect
github.com/quasilyte/gogrep v0.0.0-20220120141003-628d8b3623b5 // indirect
diff --git a/go.sum b/go.sum
index 3f9d98b50..8d4173cf6 100644
--- a/go.sum
+++ b/go.sum
@@ -888,8 +888,9 @@ github.com/prometheus/common v0.9.1/go.mod h1:yhUN8i9wzaXS3w1O07YhxHEBxD+W35wd8b
github.com/prometheus/common v0.10.0/go.mod h1:Tlit/dnDKsSWFlCLTWaA1cyBgKHSMdTB80sz/V91rCo=
github.com/prometheus/common v0.26.0/go.mod h1:M7rCNAaPfAosfx8veZJCuw84e35h3Cfd9VFqTh1DIvc=
github.com/prometheus/common v0.30.0/go.mod h1:vu+V0TpY+O6vW9J44gczi3Ap/oXXR10b+M/gUGO4Hls=
-github.com/prometheus/common v0.32.1 h1:hWIdL3N2HoUx3B8j3YN9mWor0qhY/NlEKZEaXxuIRh4=
github.com/prometheus/common v0.32.1/go.mod h1:vu+V0TpY+O6vW9J44gczi3Ap/oXXR10b+M/gUGO4Hls=
+github.com/prometheus/common v0.34.0 h1:RBmGO9d/FVjqHT0yUGQwBJhkwKV+wPCn7KGpvfab0uE=
+github.com/prometheus/common v0.34.0/go.mod h1:gB3sOl7P0TvJabZpLY5uQMpUqRCPPCyRLCZYc7JZTNE=
github.com/prometheus/procfs v0.0.0-20181005140218-185b4288413d/go.mod h1:c3At6R/oaqEKCNdg8wHV1ftS6bRYblBhIjjI8uT2IGk=
github.com/prometheus/procfs v0.0.0-20190507164030-5867b95ac084/go.mod h1:TjEm7ze935MbeOT/UhFTIMYKhuLP4wbCsTZCD3I8kEA=
github.com/prometheus/procfs v0.0.2/go.mod h1:TjEm7ze935MbeOT/UhFTIMYKhuLP4wbCsTZCD3I8kEA=
From fb7229135a2cceef615944535f466e848c7ff4e2 Mon Sep 17 00:00:00 2001
From: "dependabot[bot]" <49699333+dependabot[bot]@users.noreply.github.com>
Date: Mon, 16 May 2022 14:04:08 +0000
Subject: [PATCH 26/39] build(deps): Bump google.golang.org/grpc from 1.46.0 to
1.46.2 (#8559)
Bumps [google.golang.org/grpc](https://github.com/grpc/grpc-go) from 1.46.0 to 1.46.2.
Release notes
[](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 | 4 ++--
2 files changed, 3 insertions(+), 3 deletions(-)
diff --git a/go.mod b/go.mod
index 9af23abaa..2151f4b41 100644
--- a/go.mod
+++ b/go.mod
@@ -32,7 +32,7 @@ require (
golang.org/x/crypto v0.0.0-20220411220226-7b82a4e95df4
golang.org/x/net v0.0.0-20220412020605-290c469a71a5
golang.org/x/sync v0.0.0-20210220032951-036812b2e83c
- google.golang.org/grpc v1.46.0
+ google.golang.org/grpc v1.46.2
gopkg.in/check.v1 v1.0.0-20200902074654-038fdea0a05b // indirect
pgregory.net/rapid v0.4.7
)
diff --git a/go.sum b/go.sum
index 8d4173cf6..791fd4b73 100644
--- a/go.sum
+++ b/go.sum
@@ -1723,8 +1723,8 @@ google.golang.org/grpc v1.42.0/go.mod h1:k+4IHHFw41K8+bbowsex27ge2rCb65oeWqe4jJ5
google.golang.org/grpc v1.43.0/go.mod h1:k+4IHHFw41K8+bbowsex27ge2rCb65oeWqe4jJ590SU=
google.golang.org/grpc v1.44.0/go.mod h1:k+4IHHFw41K8+bbowsex27ge2rCb65oeWqe4jJ590SU=
google.golang.org/grpc v1.45.0/go.mod h1:lN7owxKUQEqMfSyQikvvk5tf/6zMPsrK+ONuO11+0rQ=
-google.golang.org/grpc v1.46.0 h1:oCjezcn6g6A75TGoKYBPgKmVBLexhYLM6MebdrPApP8=
-google.golang.org/grpc v1.46.0/go.mod h1:vN9eftEi1UMyUsIF80+uQXhHjbXYbm0uXoFCACuMGWk=
+google.golang.org/grpc v1.46.2 h1:u+MLGgVf7vRdjEYZ8wDFhAVNmhkbJ5hmrA1LMWK1CAQ=
+google.golang.org/grpc v1.46.2/go.mod h1:vN9eftEi1UMyUsIF80+uQXhHjbXYbm0uXoFCACuMGWk=
google.golang.org/grpc/cmd/protoc-gen-go-grpc v1.1.0/go.mod h1:6Kw0yEErY5E/yWrBtf03jp27GLLJujG4z/JK95pnjjw=
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=
From 7f79661c2efec63f37b4084083a52e2b2274a27b Mon Sep 17 00:00:00 2001
From: Sam Kleinman
Date: Mon, 16 May 2022 10:15:06 -0400
Subject: [PATCH 27/39] rfc: onboarding projects (#8413)
This is meant as a supporting recruiting document. The idea is to
describe a bunch of projects scoped and selected as teaching projects
for new engineers joining the team.
This isn't meant to replace "neweng" or "good-first-ticket" tags on
issues, but provide a higher level set of examples of the kinds of
things that someone joining the team could tackle.
---
docs/rfc/README.md | 1 +
docs/rfc/rfc-020-onboarding-projects.rst | 240 +++++++++++++++++++++++
2 files changed, 241 insertions(+)
create mode 100644 docs/rfc/rfc-020-onboarding-projects.rst
diff --git a/docs/rfc/README.md b/docs/rfc/README.md
index d944e72e7..2872c988a 100644
--- a/docs/rfc/README.md
+++ b/docs/rfc/README.md
@@ -57,5 +57,6 @@ sections.
- [RFC-017: ABCI++ Vote Extension Propagation](./rfc-017-abci++-vote-extension-propag.md)
- [RFC-018: BLS Signature Aggregation Exploration](./rfc-018-bls-agg-exploration.md)
- [RFC-019: Configuration File Versioning](./rfc-019-config-version.md)
+- [RFC-020: Onboarding Projects](./rfc-020-onboarding-projects.rst)
diff --git a/docs/rfc/rfc-020-onboarding-projects.rst b/docs/rfc/rfc-020-onboarding-projects.rst
new file mode 100644
index 000000000..dc18de65d
--- /dev/null
+++ b/docs/rfc/rfc-020-onboarding-projects.rst
@@ -0,0 +1,240 @@
+=======================================
+RFC 020: Tendermint Onboarding Projects
+=======================================
+
+.. contents::
+ :backlinks: none
+
+Changelog
+---------
+
+- 2022-03-30: Initial draft. (@tychoish)
+- 2022-04-25: Imported document to tendermint repository. (@tychoish)
+
+Overview
+--------
+
+This document describes a collection of projects that might be good for new
+engineers joining the Tendermint Core team. These projects mostly describe
+features that we'd be very excited to see land in the code base, but that are
+intentionally outside of the critical path of a release on the roadmap, and
+have the following properties that we think make good on-boarding projects:
+
+- require relatively little context for the project or its history beyond a
+ more isolated area of the code.
+
+- provide exposure to different areas of the codebase, so new team members
+ will have reason to explore the code base, build relationships with people
+ on the team, and gain experience with more than one area of the system.
+
+- be of moderate size, striking a healthy balance between trivial or
+ mechanical changes (which provide little insight) and large intractable
+ changes that require deeper insight than is available during onboarding to
+ address well. A good size project should have natural touchpoints or
+ check-ins.
+
+Projects
+--------
+
+Before diving into one of these projects, have a conversation about the
+project or aspects of Tendermint that you're excited to work on with your
+onboarding buddy. This will help make sure that these issues are still
+relevant, help you get any context, underatnding known pitfalls, and to
+confirm a high level approach or design (if relevant.) On-boarding buddies
+should be prepared to do some design work before someone joins the team.
+
+The descriptions that follow provide some basic background and attempt to
+describe the user stories and the potential impact of these project.
+
+E2E Test Systems
+~~~~~~~~~~~~~~~~
+
+Tendermint's E2E framework makes it possible to run small test networks with
+different Tendermint configurations, and make sure that the system works. The
+tests run Tendermint in a separate binary, and the system provides some very
+high level protection against making changes that could break Tendermint in
+otherwise difficult to detect ways.
+
+Working on the E2E system is a good place to get introduced to the Tendermint
+codebase, particularly for developers who are newer to Go, as the E2E
+system (generator, runner, etc.) is distinct from the rest of Tendermint and
+comparatively quite small, so it may be easier to begin making changes in this
+area. At the same time, because the E2E system exercises *all* of Tendermint,
+work in this area is a good way to get introduced to various components of the
+system.
+
+Configurable E2E Workloads
+++++++++++++++++++++++++++
+
+All E2E tests use the same workload (e.g. generated transactions, submitted to
+different nodes in the network,) which has been tuned empirically to provide a
+gentle but consistent parallel load that all E2E tests can pass. Ideally, the
+workload generator could be configurable to have different shapes of work
+(bursty, different transaction sizes, weighted to different nodes, etc.) and
+even perhaps further parameterized within a basic shape, which would make it
+possible to use our existing test infrastructure to answer different questions
+about the performance or capability of the system.
+
+The work would involve adding a new parameter to the E2E test manifest, and
+creating an option (e.g. "legacy") for the current load generation model,
+extract configurations options for the current load generation, and then
+prototype implementations of alternate load generation, and also run some
+preliminary using the tools.
+
+Byzantine E2E Workloads
++++++++++++++++++++++++
+
+There are two main kinds of integration tests in Tendermint: the E2E test
+framework, and then a collection of integration tests that masquerade as
+unit-tests. While some of this expansion of test scope is (potentially)
+inevitable, the masquerading unit tests (e.g ``consensus.byzantine_test.go``)
+end up being difficult to understand, difficult to maintain, and unreliable.
+
+One solution to this, would be to modify the E2E ABCI application to allow it
+to inject byzantine behavior, and then have this be a configurable aspect of
+a test network to be able to provoke Byzantine behavior in a "real" system and
+then observe that evidence is constructed. This would make it possible to
+remove the legacy tests entirely once the new tests have proven themselves.
+
+Abstract Orchestration Framework
+++++++++++++++++++++++++++++++++
+
+The orchestration of e2e test processes is presently done using docker
+compose, which works well, but has proven a bit limiting as all processes need
+to run on a single machine, and the log aggregation functions are confusing at
+best.
+
+This project would replace the current orchestration with something more
+generic, potentially maintaining the current system, but also allowing the e2e
+tests to manage processes using k8s. There are a few "local" k8s frameworks
+(e.g. kind and k3s,) which might be able to be useful for our current testing
+model, but hopefully, we could use this new implementation with other k8s
+systems for more flexible distribute test orchestration.
+
+Improve Operationalize Experience of ``run-multiple.sh``
+++++++++++++++++++++++++++++++++++++++++++++++++++++++++
+
+The e2e test runner currently runs a single test, and in most cases we manage
+the test cases using a shell script that ensure cleanup of entire test
+suites. This is a bit difficult to maintain and makes reproduction of test
+cases more awkward than it should be. The e2e ``runner`` itself should provide
+equivalent functionality to ``run-multiple.sh``: ensure cleanup of test cases,
+collect and process output, and be able to manage entire suites of cases.
+
+It might also be useful to implement an e2e test orchestrator that runs all
+tendermint instances in a single process, using "real" networks for faster
+feedback and iteration during development.
+
+In addition to being a bit easier to maintain, having a more capable runner
+implementation would make it easier to collect data from test runs, improve
+debugability and reporting.
+
+Fan-Out For CI E2E Tests
+++++++++++++++++++++++++
+
+While there are some parallelism in the execution of e2e tests, each e2e test
+job must build a tendermint e2e image, which takes about 5 minutes of CPU time
+per-task, which given the size of each of the runs.
+
+We'd like to be able to reduce the amount of overhead per-e2e tests while
+keeping the cycle time for working with the tests very low, while also
+maintaining a reasonable level of test coverage. This is an impossible
+tradeoff, in some ways, and the percentage of overhead at the moment is large
+enough that we can make some material progress with a moderate amount of time.
+
+Most of this work has to do with modifying github actions configuration and
+e2e artifact (docker) building to reduce redundant work. Eventually, when we
+can drop the requirement for CGo storage engines, it will be possible to move
+(cross) compile tendermint locally, and then inject the binary into the docker
+container, which would reduce a lot of the build-time complexity, although we
+can move more in this direction or have runtime flags to disable CGo
+dependencies for local development.
+
+Remove Panics
+~~~~~~~~~~~~~
+
+There are lots of places in the code base which can panic, and would not be
+particularly well handled. While in some cases, panics are the right answer,
+in many cases the panics were just added to simplify downstream error
+checking, and could easily be converted to errors.
+
+The `Don't Panic RFC
+`_
+covers some of the background and approach.
+
+While the changes are in this project are relatively rote, this will provide
+exposure to lots of different areas of the codebase as well as insight into
+how different areas of the codebase interact with eachother, as well as
+experience with the test suites and infrastructure.
+
+Implement more Expressive ABCI Applications
+~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
+
+Tendermint maintains two very simple ABCI applications (a KV application used
+for basic testing, and slightly more advanced test application used in the
+end-to-end tests). Writing an application would provide a new engineer with
+useful experiences using Tendermint that mirrors the expierence of downstream
+users.
+
+This is more of an exploratory project, but could include providing common
+interfaces on top of Tendermint consensus for other well known protocols or
+tools (e.g. ``etcd``) or a DNS server or some other tool.
+
+Self-Regulating Reactors
+~~~~~~~~~~~~~~~~~~~~~~~~
+
+Currently reactors (the internal processes that are responsible for the higher
+level behavior of Tendermint) can be started and stopped, but have no
+provision for being paused. These additional semantics may allow Tendermint to
+pause reactors (and avoid processing their messhages, etc.) and allow better
+coordination in the future.
+
+While this is a big project, it's possible to break this apart into many
+smaller projects: make p2p channels pauseable, add pause/UN-pause hooks to the
+service implementation and machinery, and finally to modify the reactor
+implementations to take advantage of these additional semantics
+
+This project would give an engineer some exposure to the p2p layer of the
+code, as well as to various aspects of the reactor implementations.
+
+Metrics
+~~~~~~~
+
+Tendermint has a metrics system that is relatively underutilized, and figuring
+out ways to capture and organize the metrics to provide value to users might
+provide an interesting set of projects for new engineers on Tendermint.
+
+Convert Logs to Metrics
++++++++++++++++++++++++
+
+Because the tendermint logs tend to be quite verbose and not particularly
+actionable, most users largely ignore the logging or run at very low
+verbosity. While the log statements in the code do describe useful events,
+taken as a whole the system is not particularly tractable, and particularly at
+the Debug level, not useful. One solution to this problem is to identify log
+messages that might be (e.g. increment a counter for certian kinds of errors)
+
+One approach might be to look at various logging statements, particularly
+debug statements or errors that are logged but not returned, and see if
+they're convertable to counters or other metrics.
+
+Expose Metrics to Tests
++++++++++++++++++++++++
+
+The existing Tendermint test suites replace the metrics infrastructure with
+no-op implementations, which means that tests can neither verify that metrics
+are ever recorded, nor can tests use metrics to observe events in the
+system. Writing an implementation, for testing, that makes it possible to
+record metrics and provides an API for introspecting this data, as well as
+potentially writing tests that take advantage of this type, could be useful.
+
+Logging Metrics
++++++++++++++++
+
+In some systems, the logging system itself can provide some interesting
+insights for operators: having metrics that track the number of messages at
+different levels as well as the total number of messages, can act as a canary
+for the system as a whole.
+
+This should be achievable by adding an interceptor layer within the logging
+package itself that can add metrics to the existing system.
From 5a42479d52f2574e40cb1c719e3ec7ac1929f9b3 Mon Sep 17 00:00:00 2001
From: "dependabot[bot]" <49699333+dependabot[bot]@users.noreply.github.com>
Date: Tue, 17 May 2022 11:55:04 +0000
Subject: [PATCH 28/39] build(deps): Bump github.com/lib/pq from 1.10.5 to
1.10.6 (#8567)
Bumps [github.com/lib/pq](https://github.com/lib/pq) from 1.10.5 to 1.10.6.
Commits
8c6de56 Merge pull request #1081 from catj-cockroach/add-kubernetes-secret-support
d8917fa adds support for kubernetes mounted private keys
54a3a4b Merge pull request #1082 from johanneswuerbach/connector-dialer
30d9faf Merge pull request #1080 from drakkan/sqlstate
[](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 | 4 ++--
2 files changed, 3 insertions(+), 3 deletions(-)
diff --git a/go.mod b/go.mod
index 2151f4b41..90142c5b1 100644
--- a/go.mod
+++ b/go.mod
@@ -16,7 +16,7 @@ require (
github.com/gorilla/websocket v1.5.0
github.com/grpc-ecosystem/go-grpc-middleware v1.3.0
github.com/grpc-ecosystem/go-grpc-prometheus v1.2.0
- github.com/lib/pq v1.10.5
+ github.com/lib/pq v1.10.6
github.com/libp2p/go-buffer-pool v0.0.2
github.com/mroth/weightedrand v0.4.1
github.com/oasisprotocol/curve25519-voi v0.0.0-20210609091139-0a56a4bca00b
diff --git a/go.sum b/go.sum
index 791fd4b73..c26a91a66 100644
--- a/go.sum
+++ b/go.sum
@@ -678,8 +678,8 @@ github.com/lib/pq v1.0.0/go.mod h1:5WUZQaWbwv1U+lTReE5YruASi9Al49XbQIvNi/34Woo=
github.com/lib/pq v1.8.0/go.mod h1:AlVN5x4E4T544tWzH6hKfbfQvm3HdbOxrmggDNAPY9o=
github.com/lib/pq v1.9.0/go.mod h1:AlVN5x4E4T544tWzH6hKfbfQvm3HdbOxrmggDNAPY9o=
github.com/lib/pq v1.10.4/go.mod h1:AlVN5x4E4T544tWzH6hKfbfQvm3HdbOxrmggDNAPY9o=
-github.com/lib/pq v1.10.5 h1:J+gdV2cUmX7ZqL2B0lFcW0m+egaHC2V3lpO8nWxyYiQ=
-github.com/lib/pq v1.10.5/go.mod h1:AlVN5x4E4T544tWzH6hKfbfQvm3HdbOxrmggDNAPY9o=
+github.com/lib/pq v1.10.6 h1:jbk+ZieJ0D7EVGJYpL9QTz7/YW6UHbmdnZWYyK5cdBs=
+github.com/lib/pq v1.10.6/go.mod h1:AlVN5x4E4T544tWzH6hKfbfQvm3HdbOxrmggDNAPY9o=
github.com/libp2p/go-buffer-pool v0.0.2 h1:QNK2iAFa8gjAe1SPz6mHSMuCcjs+X1wlHzeOSqcmlfs=
github.com/libp2p/go-buffer-pool v0.0.2/go.mod h1:MvaB6xw5vOrDl8rYZGLFdKAuk/hRoRZd1Vi32+RXyFM=
github.com/lufeee/execinquery v1.0.0 h1:1XUTuLIVPDlFvUU3LXmmZwHDsolsxXnY67lzhpeqe0I=
From 2897b75853818f72930499e25b77b8dc8fdff7e8 Mon Sep 17 00:00:00 2001
From: Sam Kleinman
Date: Tue, 17 May 2022 10:56:26 -0400
Subject: [PATCH 29/39] p2p: remove unused get height methods (#8569)
---
internal/mempool/reactor.go | 40 ++++++++------------------------
internal/mempool/reactor_test.go | 1 -
internal/p2p/peermanager.go | 31 -------------------------
internal/p2p/peermanager_test.go | 35 ----------------------------
node/node.go | 2 +-
node/setup.go | 2 --
6 files changed, 11 insertions(+), 100 deletions(-)
diff --git a/internal/mempool/reactor.go b/internal/mempool/reactor.go
index 3c22988ee..28ee9e334 100644
--- a/internal/mempool/reactor.go
+++ b/internal/mempool/reactor.go
@@ -6,7 +6,6 @@ import (
"fmt"
"runtime/debug"
"sync"
- "time"
"github.com/tendermint/tendermint/config"
"github.com/tendermint/tendermint/internal/libs/clist"
@@ -22,13 +21,6 @@ var (
_ p2p.Wrapper = (*protomem.Message)(nil)
)
-// PeerManager defines the interface contract required for getting necessary
-// peer information. This should eventually be replaced with a message-oriented
-// approach utilizing the p2p stack.
-type PeerManager interface {
- GetHeight(types.NodeID) int64
-}
-
// Reactor implements a service that contains mempool of txs that are broadcasted
// amongst peers. It maintains a map from peer ID to counter, to prevent gossiping
// txs to the peers you received it from.
@@ -40,9 +32,8 @@ type Reactor struct {
mempool *TxMempool
ids *IDs
- getPeerHeight func(types.NodeID) int64
- peerEvents p2p.PeerEventSubscriber
- chCreator p2p.ChannelCreator
+ peerEvents p2p.PeerEventSubscriber
+ chCreator p2p.ChannelCreator
// observePanic is a function for observing panics that were recovered in methods on
// Reactor. observePanic is called with the recovered value.
@@ -59,18 +50,16 @@ func NewReactor(
txmp *TxMempool,
chCreator p2p.ChannelCreator,
peerEvents p2p.PeerEventSubscriber,
- getPeerHeight func(types.NodeID) int64,
) *Reactor {
r := &Reactor{
- logger: logger,
- cfg: cfg,
- mempool: txmp,
- ids: NewMempoolIDs(),
- chCreator: chCreator,
- peerEvents: peerEvents,
- getPeerHeight: getPeerHeight,
- peerRoutines: make(map[types.NodeID]context.CancelFunc),
- observePanic: defaultObservePanic,
+ logger: logger,
+ cfg: cfg,
+ mempool: txmp,
+ ids: NewMempoolIDs(),
+ chCreator: chCreator,
+ peerEvents: peerEvents,
+ peerRoutines: make(map[types.NodeID]context.CancelFunc),
+ observePanic: defaultObservePanic,
}
r.BaseService = *service.NewBaseService(logger, "Mempool", r)
@@ -327,15 +316,6 @@ func (r *Reactor) broadcastTxRoutine(ctx context.Context, peerID types.NodeID, m
memTx := nextGossipTx.Value.(*WrappedTx)
- if r.getPeerHeight != nil {
- height := r.getPeerHeight(peerID)
- if height > 0 && height < memTx.height-1 {
- // allow for a lag of one block
- time.Sleep(PeerCatchupSleepIntervalMS * time.Millisecond)
- continue
- }
- }
-
// NOTE: Transaction batching was disabled due to:
// https://github.com/tendermint/tendermint/issues/5796
if ok := r.mempool.txStore.TxHasPeer(memTx.hash, peerMempoolID); !ok {
diff --git a/internal/mempool/reactor_test.go b/internal/mempool/reactor_test.go
index 8ceae2013..351315bae 100644
--- a/internal/mempool/reactor_test.go
+++ b/internal/mempool/reactor_test.go
@@ -85,7 +85,6 @@ func setupReactors(ctx context.Context, t *testing.T, logger log.Logger, numNode
mempool,
chCreator,
func(ctx context.Context) *p2p.PeerUpdates { return rts.peerUpdates[nodeID] },
- rts.network.Nodes[nodeID].PeerManager.GetHeight,
)
rts.nodes = append(rts.nodes, nodeID)
diff --git a/internal/p2p/peermanager.go b/internal/p2p/peermanager.go
index 756551a49..165b00e61 100644
--- a/internal/p2p/peermanager.go
+++ b/internal/p2p/peermanager.go
@@ -1027,37 +1027,6 @@ func (m *PeerManager) retryDelay(failures uint32, persistent bool) time.Duration
return delay
}
-// GetHeight returns a peer's height, as reported via SetHeight, or 0 if the
-// peer or height is unknown.
-//
-// FIXME: This is a temporary workaround to share state between the consensus
-// and mempool reactors, carried over from the legacy P2P stack. Reactors should
-// not have dependencies on each other, instead tracking this themselves.
-func (m *PeerManager) GetHeight(peerID types.NodeID) int64 {
- m.mtx.Lock()
- defer m.mtx.Unlock()
-
- peer, _ := m.store.Get(peerID)
- return peer.Height
-}
-
-// SetHeight stores a peer's height, making it available via GetHeight.
-//
-// FIXME: This is a temporary workaround to share state between the consensus
-// and mempool reactors, carried over from the legacy P2P stack. Reactors should
-// not have dependencies on each other, instead tracking this themselves.
-func (m *PeerManager) SetHeight(peerID types.NodeID, height int64) error {
- m.mtx.Lock()
- defer m.mtx.Unlock()
-
- peer, ok := m.store.Get(peerID)
- if !ok {
- peer = m.newPeerInfo(peerID)
- }
- peer.Height = height
- return m.store.Set(peer)
-}
-
// peerStore stores information about peers. It is not thread-safe, assuming it
// is only used by PeerManager which handles concurrency control. This allows
// the manager to execute multiple operations atomically via its own mutex.
diff --git a/internal/p2p/peermanager_test.go b/internal/p2p/peermanager_test.go
index 82d1e2693..47e8462a4 100644
--- a/internal/p2p/peermanager_test.go
+++ b/internal/p2p/peermanager_test.go
@@ -1868,38 +1868,3 @@ func TestPeerManager_Advertise_Self(t *testing.T) {
self,
}, peerManager.Advertise(dID, 100))
}
-
-func TestPeerManager_SetHeight_GetHeight(t *testing.T) {
- a := p2p.NodeAddress{Protocol: "memory", NodeID: types.NodeID(strings.Repeat("a", 40))}
- b := p2p.NodeAddress{Protocol: "memory", NodeID: types.NodeID(strings.Repeat("b", 40))}
-
- db := dbm.NewMemDB()
- peerManager, err := p2p.NewPeerManager(selfID, db, p2p.PeerManagerOptions{})
- require.NoError(t, err)
-
- // Getting a height should default to 0, for unknown peers and
- // for known peers without height.
- added, err := peerManager.Add(a)
- require.NoError(t, err)
- require.True(t, added)
- require.EqualValues(t, 0, peerManager.GetHeight(a.NodeID))
- require.EqualValues(t, 0, peerManager.GetHeight(b.NodeID))
-
- // Setting a height should work for a known node.
- require.NoError(t, peerManager.SetHeight(a.NodeID, 3))
- require.EqualValues(t, 3, peerManager.GetHeight(a.NodeID))
-
- // Setting a height should add an unknown node.
- require.Equal(t, []types.NodeID{a.NodeID}, peerManager.Peers())
- require.NoError(t, peerManager.SetHeight(b.NodeID, 7))
- require.EqualValues(t, 7, peerManager.GetHeight(b.NodeID))
- require.ElementsMatch(t, []types.NodeID{a.NodeID, b.NodeID}, peerManager.Peers())
-
- // The heights should not be persisted.
- peerManager, err = p2p.NewPeerManager(selfID, db, p2p.PeerManagerOptions{})
- require.NoError(t, err)
-
- require.ElementsMatch(t, []types.NodeID{a.NodeID, b.NodeID}, peerManager.Peers())
- require.Zero(t, peerManager.GetHeight(a.NodeID))
- require.Zero(t, peerManager.GetHeight(b.NodeID))
-}
diff --git a/node/node.go b/node/node.go
index 56379d2e2..1bda1f0f7 100644
--- a/node/node.go
+++ b/node/node.go
@@ -266,7 +266,7 @@ func makeNode(
node.evPool = evPool
mpReactor, mp := createMempoolReactor(logger, cfg, proxyApp, stateStore, nodeMetrics.mempool,
- peerManager.Subscribe, node.router.OpenChannel, peerManager.GetHeight)
+ peerManager.Subscribe, node.router.OpenChannel)
node.rpcEnv.Mempool = mp
node.services = append(node.services, mpReactor)
diff --git a/node/setup.go b/node/setup.go
index d6966800a..8089ea466 100644
--- a/node/setup.go
+++ b/node/setup.go
@@ -147,7 +147,6 @@ func createMempoolReactor(
memplMetrics *mempool.Metrics,
peerEvents p2p.PeerEventSubscriber,
chCreator p2p.ChannelCreator,
- peerHeight func(types.NodeID) int64,
) (service.Service, mempool.Mempool) {
logger = logger.With("module", "mempool")
@@ -166,7 +165,6 @@ func createMempoolReactor(
mp,
chCreator,
peerEvents,
- peerHeight,
)
if cfg.Consensus.WaitForTxs() {
From 66c4c82f7a687f75d2641a2452222b21a8d7d7ac Mon Sep 17 00:00:00 2001
From: "M. J. Fromberger"
Date: Tue, 17 May 2022 08:52:39 -0700
Subject: [PATCH 30/39] rpc: rework timeouts to be per-method instead of global
(#8570)
* rpc: rework timeouts to be per-method instead of global
Prior to this change, we set a 10-second global timeout for all RPC methods
using the net/http Server type's WriteTimeout. This meant that any request
whose handler did not return within that period would simply drop the
connection to the client.
This timeout is too short for a default, as evidenced by issues like [1] and
[2]. In addition, the mode of failure on the client side is confusing; it
shows up as a dropped connection (EOF) rather than a meaningful error from the
service. More importantly, various methods have diffent constraints: Some
should be able to return quickly, others may need to adjust based on the
application workload.
This is a first step toward supporting configurable timeouts. This change:
- Removes the server-wide default global timeout, and instead:
- Wires up a default context timeout for all RPC handlers.
- Increases the default timeout from 10s to 60s.
- Adds a hook to override this per-method as needed.
This does NOT expose the timeouts in the configuration file (yet).
[1] https://github.com/osmosis-labs/osmosis/issues/1391
[2] https://github.com/tendermint/tendermint/issues/8465
---
CHANGELOG_PENDING.md | 1 +
internal/rpc/core/routes.go | 2 +-
rpc/jsonrpc/server/http_server.go | 29 +++++++++++++++-------
rpc/jsonrpc/server/rpc_func.go | 40 +++++++++++++++++++++++--------
4 files changed, 52 insertions(+), 20 deletions(-)
diff --git a/CHANGELOG_PENDING.md b/CHANGELOG_PENDING.md
index 65ab5ee3b..d38caf50b 100644
--- a/CHANGELOG_PENDING.md
+++ b/CHANGELOG_PENDING.md
@@ -21,6 +21,7 @@ Special thanks to external contributors on this release:
- [rpc] \#7982 Add new Events interface and deprecate Subscribe. (@creachadair)
- [cli] \#8081 make the reset command safe to use by intoducing `reset-state` command. Fixed by \#8259. (@marbar3778, @cmwaters)
- [config] \#8222 default indexer configuration to null. (@creachadair)
+ - [rpc] \#8570 rework timeouts to be per-method instead of global. (@creachadair)
- Apps
diff --git a/internal/rpc/core/routes.go b/internal/rpc/core/routes.go
index 4bc1ca414..cafb92094 100644
--- a/internal/rpc/core/routes.go
+++ b/internal/rpc/core/routes.go
@@ -28,7 +28,7 @@ func NewRoutesMap(svc RPCService, opts *RouteOptions) RoutesMap {
out := RoutesMap{
// Event subscription. Note that subscribe, unsubscribe, and
// unsubscribe_all are only available via the websocket endpoint.
- "events": rpc.NewRPCFunc(svc.Events),
+ "events": rpc.NewRPCFunc(svc.Events).Timeout(0),
"subscribe": rpc.NewWSRPCFunc(svc.Subscribe),
"unsubscribe": rpc.NewWSRPCFunc(svc.Unsubscribe),
"unsubscribe_all": rpc.NewWSRPCFunc(svc.UnsubscribeAll),
diff --git a/rpc/jsonrpc/server/http_server.go b/rpc/jsonrpc/server/http_server.go
index 0b715835d..50a37158e 100644
--- a/rpc/jsonrpc/server/http_server.go
+++ b/rpc/jsonrpc/server/http_server.go
@@ -20,16 +20,27 @@ import (
// Config is a RPC server configuration.
type Config struct {
- // see netutil.LimitListener
+ // The maximum number of connections that will be accepted by the listener.
+ // See https://godoc.org/golang.org/x/net/netutil#LimitListener
MaxOpenConnections int
- // mirrors http.Server#ReadTimeout
+
+ // Used to set the HTTP server's per-request read timeout.
+ // See https://godoc.org/net/http#Server.ReadTimeout
ReadTimeout time.Duration
- // mirrors http.Server#WriteTimeout
+
+ // Used to set the HTTP server's per-request write timeout. Note that this
+ // affects ALL methods on the server, so it should not be set too low. This
+ // should be used as a safety valve, not a resource-control timeout.
+ //
+ // See https://godoc.org/net/http#Server.WriteTimeout
WriteTimeout time.Duration
- // MaxBodyBytes controls the maximum number of bytes the
- // server will read parsing the request body.
+
+ // Controls the maximum number of bytes the server will read parsing the
+ // request body.
MaxBodyBytes int64
- // mirrors http.Server#MaxHeaderBytes
+
+ // Controls the maximum size of a request header.
+ // See https://godoc.org/net/http#Server.MaxHeaderBytes
MaxHeaderBytes int
}
@@ -38,9 +49,9 @@ func DefaultConfig() *Config {
return &Config{
MaxOpenConnections: 0, // unlimited
ReadTimeout: 10 * time.Second,
- WriteTimeout: 10 * time.Second,
- MaxBodyBytes: int64(1000000), // 1MB
- MaxHeaderBytes: 1 << 20, // same as the net/http default
+ WriteTimeout: 0, // no default timeout
+ MaxBodyBytes: 1000000, // 1MB
+ MaxHeaderBytes: 1 << 20, // same as the net/http default
}
}
diff --git a/rpc/jsonrpc/server/rpc_func.go b/rpc/jsonrpc/server/rpc_func.go
index 8eba28728..1fff323d7 100644
--- a/rpc/jsonrpc/server/rpc_func.go
+++ b/rpc/jsonrpc/server/rpc_func.go
@@ -9,11 +9,16 @@ import (
"net/http"
"reflect"
"strings"
+ "time"
"github.com/tendermint/tendermint/libs/log"
rpctypes "github.com/tendermint/tendermint/rpc/jsonrpc/types"
)
+// DefaultRPCTimeout is the default context timeout for calls to any RPC method
+// that does not override it with a more specific timeout.
+const DefaultRPCTimeout = 60 * time.Second
+
// RegisterRPCFuncs adds a route to mux for each non-websocket function in the
// funcMap, and also a root JSON-RPC POST handler.
func RegisterRPCFuncs(mux *http.ServeMux, funcMap map[string]*RPCFunc, logger log.Logger) {
@@ -32,11 +37,12 @@ func RegisterRPCFuncs(mux *http.ServeMux, funcMap map[string]*RPCFunc, logger lo
// RPCFunc contains the introspected type information for a function.
type RPCFunc struct {
- f reflect.Value // underlying rpc function
- param reflect.Type // the parameter struct, or nil
- result reflect.Type // the non-error result type, or nil
- args []argInfo // names and type information (for URL decoding)
- ws bool // websocket only
+ f reflect.Value // underlying rpc function
+ param reflect.Type // the parameter struct, or nil
+ result reflect.Type // the non-error result type, or nil
+ args []argInfo // names and type information (for URL decoding)
+ timeout time.Duration // default request timeout, 0 means none
+ ws bool // websocket only
}
// argInfo records the name of a field, along with a bit to tell whether the
@@ -52,6 +58,12 @@ type argInfo struct {
// with the resulting argument value. It reports an error if parameter parsing
// fails, otherwise it returns the result from the wrapped function.
func (rf *RPCFunc) Call(ctx context.Context, params json.RawMessage) (interface{}, error) {
+ // If ctx has its own deadline we will respect it; otherwise use rf.timeout.
+ if _, ok := ctx.Deadline(); !ok && rf.timeout > 0 {
+ var cancel context.CancelFunc
+ ctx, cancel = context.WithTimeout(ctx, rf.timeout)
+ defer cancel()
+ }
args, err := rf.parseParams(ctx, params)
if err != nil {
return nil, err
@@ -74,6 +86,11 @@ func (rf *RPCFunc) Call(ctx context.Context, params json.RawMessage) (interface{
return returns[0].Interface(), nil
}
+// Timeout updates rf to include a default timeout for calls to rf. This
+// timeout is used if one is not already provided on the request context.
+// Setting d == 0 means there will be no timeout. Returns rf to allow chaining.
+func (rf *RPCFunc) Timeout(d time.Duration) *RPCFunc { rf.timeout = d; return rf }
+
// parseParams parses the parameters of a JSON-RPC request and returns the
// corresponding argument values. On success, the first argument value will be
// the value of ctx.
@@ -129,7 +146,9 @@ func (rf *RPCFunc) adjustParams(data []byte) (json.RawMessage, error) {
// func(context.Context, *T) (R, error)
//
// for an arbitrary struct type T and type R. NewRPCFunc will panic if f does
-// not have one of these forms.
+// not have one of these forms. A newly-constructed RPCFunc has a default
+// timeout of DefaultRPCTimeout; use the Timeout method to adjust this as
+// needed.
func NewRPCFunc(f interface{}) *RPCFunc {
rf, err := newRPCFunc(f)
if err != nil {
@@ -215,10 +234,11 @@ func newRPCFunc(f interface{}) (*RPCFunc, error) {
}
return &RPCFunc{
- f: fv,
- param: ptype,
- result: rtype,
- args: args,
+ f: fv,
+ param: ptype,
+ result: rtype,
+ args: args,
+ timeout: DefaultRPCTimeout, // until overridden
}, nil
}
From 21f140410bdfd3f097f9e563cf6b714f08ff5ca5 Mon Sep 17 00:00:00 2001
From: "M. J. Fromberger"
Date: Tue, 17 May 2022 09:49:23 -0700
Subject: [PATCH 31/39] rpc: enable the ADR 075 event log by default in new
configs (#8572)
Since we are deprecating the stream-based event subscription in v0.36, we
should ensure that new nodes enable the replacement by default. For now, just
set a baseline 30-second window.
---
config/config.go | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/config/config.go b/config/config.go
index 500e3f7d6..c1fa4223a 100644
--- a/config/config.go
+++ b/config/config.go
@@ -523,7 +523,7 @@ func DefaultRPCConfig() *RPCConfig {
MaxSubscriptionClients: 100,
MaxSubscriptionsPerClient: 5,
ExperimentalDisableWebsocket: false, // compatible with TM v0.35 and earlier
- EventLogWindowSize: 0, // disables /events RPC by default
+ EventLogWindowSize: 30 * time.Second,
EventLogMaxItems: 0,
TimeoutBroadcastTxCommit: 10 * time.Second,
From c620900fdd98838e29debdc47243e03f13d3ef05 Mon Sep 17 00:00:00 2001
From: "M. J. Fromberger"
Date: Tue, 17 May 2022 10:34:43 -0700
Subject: [PATCH 32/39] rpc: fix plumbing of broadcast_tx_commit timeout
(#8573)
In #3435 we allowed this timeout to override the global write timeout.
But after #8570 this meant we were applying a shorter timeout by default.
Don't do the patch if the timeout is already unlimited.
This is a temporary workaround; in light of #8561 I plan to get rid of this
option entirely during the v0.37 cycle, but meanwhile we should keep existing
use more or less coherent.
---
cmd/tendermint/commands/light.go | 3 ++-
internal/inspect/rpc/rpc.go | 3 ++-
internal/rpc/core/env.go | 3 ++-
test/e2e/node/main.go | 3 ++-
4 files changed, 8 insertions(+), 4 deletions(-)
diff --git a/cmd/tendermint/commands/light.go b/cmd/tendermint/commands/light.go
index 8e39d7900..2b812fe18 100644
--- a/cmd/tendermint/commands/light.go
+++ b/cmd/tendermint/commands/light.go
@@ -171,7 +171,8 @@ for applications built w/ Cosmos SDK).
// If necessary adjust global WriteTimeout to ensure it's greater than
// TimeoutBroadcastTxCommit.
// See https://github.com/tendermint/tendermint/issues/3435
- if cfg.WriteTimeout <= conf.RPC.TimeoutBroadcastTxCommit {
+ // Note we don't need to adjust anything if the timeout is already unlimited.
+ if cfg.WriteTimeout > 0 && cfg.WriteTimeout <= conf.RPC.TimeoutBroadcastTxCommit {
cfg.WriteTimeout = conf.RPC.TimeoutBroadcastTxCommit + 1*time.Second
}
diff --git a/internal/inspect/rpc/rpc.go b/internal/inspect/rpc/rpc.go
index 00c3e52ef..d70616834 100644
--- a/internal/inspect/rpc/rpc.go
+++ b/internal/inspect/rpc/rpc.go
@@ -125,7 +125,8 @@ func serverRPCConfig(r *config.RPCConfig) *server.Config {
// If necessary adjust global WriteTimeout to ensure it's greater than
// TimeoutBroadcastTxCommit.
// See https://github.com/tendermint/tendermint/issues/3435
- if cfg.WriteTimeout <= r.TimeoutBroadcastTxCommit {
+ // Note we don't need to adjust anything if the timeout is already unlimited.
+ if cfg.WriteTimeout > 0 && cfg.WriteTimeout <= r.TimeoutBroadcastTxCommit {
cfg.WriteTimeout = r.TimeoutBroadcastTxCommit + 1*time.Second
}
return cfg
diff --git a/internal/rpc/core/env.go b/internal/rpc/core/env.go
index 24f43a4a7..124525f26 100644
--- a/internal/rpc/core/env.go
+++ b/internal/rpc/core/env.go
@@ -236,7 +236,8 @@ func (env *Environment) StartService(ctx context.Context, conf *config.Config) (
// If necessary adjust global WriteTimeout to ensure it's greater than
// TimeoutBroadcastTxCommit.
// See https://github.com/tendermint/tendermint/issues/3435
- if cfg.WriteTimeout <= conf.RPC.TimeoutBroadcastTxCommit {
+ // Note we don't need to adjust anything if the timeout is already unlimited.
+ if cfg.WriteTimeout > 0 && cfg.WriteTimeout <= conf.RPC.TimeoutBroadcastTxCommit {
cfg.WriteTimeout = conf.RPC.TimeoutBroadcastTxCommit + 1*time.Second
}
diff --git a/test/e2e/node/main.go b/test/e2e/node/main.go
index 2cbb9e4b0..94c1af1ab 100644
--- a/test/e2e/node/main.go
+++ b/test/e2e/node/main.go
@@ -210,7 +210,8 @@ func startLightNode(ctx context.Context, logger log.Logger, cfg *Config) error {
// If necessary adjust global WriteTimeout to ensure it's greater than
// TimeoutBroadcastTxCommit.
// See https://github.com/tendermint/tendermint/issues/3435
- if rpccfg.WriteTimeout <= tmcfg.RPC.TimeoutBroadcastTxCommit {
+ // Note we don't need to adjust anything if the timeout is already unlimited.
+ if rpccfg.WriteTimeout > 0 && rpccfg.WriteTimeout <= tmcfg.RPC.TimeoutBroadcastTxCommit {
rpccfg.WriteTimeout = tmcfg.RPC.TimeoutBroadcastTxCommit + 1*time.Second
}
From 2e20b820ab6cf009c12a1cba74507b2a8bf4d9b4 Mon Sep 17 00:00:00 2001
From: Sergio Mena
Date: Thu, 19 May 2022 08:53:28 +0200
Subject: [PATCH 33/39] Adapted `apps.md` from ABCI directory (#8506)
* Copied over the 'Apps' section from ABCI. Need to adapt it
* Adapted the ABCI text in requirements section
* Update spec/abci++/abci++_app_requirements_002_draft.md
Co-authored-by: Daniel
* Update spec/abci++/abci++_app_requirements_002_draft.md
Co-authored-by: Daniel
* Update spec/abci++/abci++_app_requirements_002_draft.md
Co-authored-by: Daniel
* Update spec/abci++/abci++_app_requirements_002_draft.md
Co-authored-by: Daniel
* Update spec/abci++/abci++_app_requirements_002_draft.md
Co-authored-by: Daniel
* Update spec/abci++/abci++_app_requirements_002_draft.md
Co-authored-by: Daniel
* Update spec/abci++/abci++_app_requirements_002_draft.md
Co-authored-by: Daniel
* Update spec/abci++/abci++_app_requirements_002_draft.md
Co-authored-by: Daniel
* Update spec/abci++/abci++_app_requirements_002_draft.md
Co-authored-by: Daniel
* Update spec/abci++/abci++_app_requirements_002_draft.md
Co-authored-by: Daniel
* Update spec/abci++/abci++_app_requirements_002_draft.md
Co-authored-by: Daniel
* Update spec/abci++/abci++_app_requirements_002_draft.md
Co-authored-by: Daniel
* Update spec/abci++/abci++_app_requirements_002_draft.md
Co-authored-by: Daniel
* Update spec/abci++/abci++_app_requirements_002_draft.md
Co-authored-by: Daniel
* Update spec/abci++/abci++_app_requirements_002_draft.md
Co-authored-by: Daniel
* Update spec/abci++/abci++_app_requirements_002_draft.md
Co-authored-by: Daniel
* Update spec/abci++/abci++_app_requirements_002_draft.md
Co-authored-by: Daniel
* Update spec/abci++/abci++_app_requirements_002_draft.md
Co-authored-by: Daniel
* Update spec/abci++/abci++_app_requirements_002_draft.md
Co-authored-by: Daniel
* Update spec/abci++/abci++_app_requirements_002_draft.md
Co-authored-by: Daniel
* Update spec/abci++/abci++_app_requirements_002_draft.md
Co-authored-by: Daniel
* Update spec/abci++/abci++_app_requirements_002_draft.md
Co-authored-by: Daniel
* Update spec/abci++/abci++_app_requirements_002_draft.md
Co-authored-by: Daniel
* Update spec/abci++/abci++_app_requirements_002_draft.md
Co-authored-by: Daniel
* Update spec/abci++/abci++_app_requirements_002_draft.md
Co-authored-by: Daniel
* Update spec/abci++/abci++_app_requirements_002_draft.md
Co-authored-by: Daniel
* Update spec/abci++/abci++_app_requirements_002_draft.md
Co-authored-by: Daniel
* Update spec/abci++/abci++_app_requirements_002_draft.md
Co-authored-by: Daniel
* Update spec/abci++/abci++_app_requirements_002_draft.md
Co-authored-by: Daniel
* Update spec/abci++/abci++_app_requirements_002_draft.md
Co-authored-by: Daniel
* Update spec/abci++/abci++_app_requirements_002_draft.md
Co-authored-by: Daniel
* Update spec/abci++/abci++_app_requirements_002_draft.md
Co-authored-by: M. J. Fromberger
* Adressed @cason's comments
* Update spec/abci++/abci++_app_requirements_002_draft.md
Co-authored-by: M. J. Fromberger
* Update spec/abci++/abci++_app_requirements_002_draft.md
Co-authored-by: M. J. Fromberger
* Update spec/abci++/abci++_app_requirements_002_draft.md
Co-authored-by: M. J. Fromberger
* Update spec/abci++/abci++_app_requirements_002_draft.md
Co-authored-by: M. J. Fromberger
* Addressed remaining comments
* Addressed some of @cmwaters comments
* Addressed more comments
* Addressed @JayT106's comments
Co-authored-by: Daniel
Co-authored-by: M. J. Fromberger
---
spec/abci++/README.md | 2 +-
.../abci++_app_requirements_002_draft.md | 845 ++++++++++++++++++
spec/abci++/abci++_methods_002_draft.md | 20 +-
3 files changed, 857 insertions(+), 10 deletions(-)
diff --git a/spec/abci++/README.md b/spec/abci++/README.md
index 38feba9d7..0f7a87c4a 100644
--- a/spec/abci++/README.md
+++ b/spec/abci++/README.md
@@ -29,7 +29,7 @@ This specification is split as follows:
- [Methods](./abci++_methods_002_draft.md) - complete details on all ABCI++ methods
and message types.
- [Requirements for the Application](./abci++_app_requirements_002_draft.md) - formal requirements
- on the Application's logic to ensure liveness of Tendermint. These requirements define what
+ on the Application's logic to ensure Tendermint properties such as liveness. These requirements define what
Tendermint expects from the Application.
- [Tendermint's expected behavior](./abci++_tmint_expected_behavior_002_draft.md) - specification of
how the different ABCI++ methods may be called by Tendermint. This explains what the Application
diff --git a/spec/abci++/abci++_app_requirements_002_draft.md b/spec/abci++/abci++_app_requirements_002_draft.md
index 68014a536..ff9df2c56 100644
--- a/spec/abci++/abci++_app_requirements_002_draft.md
+++ b/spec/abci++/abci++_app_requirements_002_draft.md
@@ -5,6 +5,8 @@ title: Application Requirements
# Application Requirements
+## Formal Requirements
+
This section specifies what Tendermint expects from the Application. It is structured as a set
of formal requirements that can be used for testing and verification of the Application's logic.
@@ -177,3 +179,846 @@ Likewise, `ExtendVote` can also be non-deterministic:
but may also depend on other values or operations.
* *wrp = wrq ⇏
erp = erq*
+
+## Managing the Application state and related topics
+
+### Connection State
+
+Tendermint maintains four concurrent ABCI++ connections, namely
+[Consensus Connection](#consensus-connection),
+[Mempool Connection](#mempool-connection),
+[Info/Query Connection](#infoquery-connection), and
+[Snapshot Connection](#snapshot-connection).
+It is common for an application to maintain a distinct copy of
+the state for each connection, which are synchronized upon `Commit` calls.
+
+#### Concurrency
+
+In principle, each of the four ABCI++ connections operates concurrently with one
+another. This means applications need to ensure access to state is
+thread safe. Up to v0.35.x, both the
+[default in-process ABCI client](https://github.com/tendermint/tendermint/blob/v0.35.x/abci/client/local_client.go#L18)
+and the
+[default Go ABCI server](https://github.com/tendermint/tendermint/blob/v0.35.x/abci/server/socket_server.go#L32)
+used a global lock to guard the handling of events across all connections, so they were not
+concurrent at all. This meant whether your app was compiled in-process with
+Tendermint using the `NewLocalClient`, or run out-of-process using the `SocketServer`,
+ABCI messages from all connections were received in sequence, one at a
+time.
+This is no longer the case starting from v0.36.0: the global locks have been removed and it is
+up to the Application to synchronize access to its state when handling
+ABCI++ methods on all connections.
+Nevertheless, as all ABCI calls are now synchronous, ABCI messages using the same connection are
+still received in sequence.
+
+#### FinalizeBlock
+
+When the consensus algorithm decides on a block, Tendermint uses `FinalizeBlock` to send the
+decided block's data to the Application, which uses it to transition its state.
+
+The Application must remember the latest height from which it
+has run a successful `Commit` so that it can tell Tendermint where to
+pick up from when it recovers from a crash. See information on the Handshake
+[here](#crash-recovery).
+
+#### Commit
+
+The Application should persist its state during `Commit`, before returning from it.
+
+Before invoking `Commit`, Tendermint locks the mempool and flushes the mempool connection. This ensures that
+no new messages
+will be received on the mempool connection during this processing step, providing an opportunity to safely
+update all four
+connection states to the latest committed state at the same time.
+
+When `Commit` returns, Tendermint unlocks the mempool.
+
+WARNING: if the ABCI app logic processing the `Commit` message sends a
+`/broadcast_tx_sync` or `/broadcast_tx` and waits for the response
+before proceeding, it will deadlock. Executing `broadcast_tx` calls
+involves acquiring the mempool lock that Tendermint holds during the `Commit` call.
+Synchronous mempool-related calls must be avoided as part of the sequential logic of the
+`Commit` function.
+
+#### Candidate States
+
+Tendermint calls `PrepareProposal` when it is about to send a proposed block to the network.
+Likewise, Tendermint calls `ProcessProposal` upon reception of a proposed block from the
+network. In both cases, the proposed block's data
+is disclosed to the Application, in the same conditions as is done in `FinalizeBlock`.
+The block data disclosed the to Application by these three methods are the following:
+
+* the transaction list
+* the `LastCommit` referring to the previous block
+* the block header's hash (except in `PrepareProposal`, where it is not known yet)
+* list of validators that misbehaved
+* the block's timestamp
+* `NextValidatorsHash`
+* Proposer address
+
+The Application may decide to *immediately* execute the given block (i.e., upon `PrepareProposal`
+or `ProcessProposal`). There are two main reasons why the Application may want to do this:
+
+* *Avoiding invalid transactions in blocks*.
+ In order to be sure that the block does not contain *any* invalid transaction, there may be
+ no way other than fully executing the transactions in the block as though it was the *decided*
+ block.
+* *Quick `FinalizeBlock` execution*.
+ Upon reception of the decided block via `FinalizeBlock`, if that same block was executed
+ upon `PrepareProposal` or `ProcessProposal` and the resulting state was kept in memory, the
+ Application can simply apply that state (faster) to the main state, rather than reexecuting
+ the decided block (slower).
+
+`PrepareProposal`/`ProcessProposal` can be called many times for a given height. Moreover,
+it is not possible to accurately predict which of the blocks proposed in a height will be decided,
+being delivered to the Application in that height's `FinalizeBlock`.
+Therefore, the state resulting from executing a proposed block, denoted a *candidate state*, should
+be kept in memory as a possible final state for that height. When `FinalizeBlock` is called, the Application should
+check if the decided block corresponds to one of its candidate states; if so, it will apply it as
+its *ExecuteTxState* (see [Consensus Connection](#consensus-connection) below),
+which will be persisted during the upcoming `Commit` call.
+
+Under adverse conditions (e.g., network instability), Tendermint might take many rounds.
+In this case, potentially many proposed blocks will be disclosed to the Application for a given height.
+By the nature of Tendermint's consensus algorithm, the number of proposed blocks received by the Application
+for a particular height cannot be bound, so Application developers must act with care and use mechanisms
+to bound memory usage. As a general rule, the Application should be ready to discard candidate states
+before `FinalizeBlock`, even if one of them might end up corresponding to the
+decided block and thus have to be reexecuted upon `FinalizeBlock`.
+
+### States and ABCI++ Connections
+
+#### Consensus Connection
+
+The Consensus Connection should maintain an *ExecuteTxState* — the working state
+for block execution. It should be updated by the call to `FinalizeBlock`
+during block execution and committed to disk as the "latest
+committed state" during `Commit`. Execution of a proposed block (via `PrepareProposal`/`ProcessProposal`)
+**must not** update the *ExecuteTxState*, but rather be kept as a separate candidate state until `FinalizeBlock`
+confirms which of the candidate states (if any) can be used to update *ExecuteTxState*.
+
+#### Mempool Connection
+
+The mempool Connection maintains *CheckTxState*. Tendermint sequentially processes an incoming
+transaction (via RPC from client or P2P from the gossip layer) against *CheckTxState*.
+If the processing does not return any error, the transaction is accepted into the mempool
+and Tendermint starts gossipping it.
+*CheckTxState* should be reset to the latest committed state
+at the end of every `Commit`.
+
+During the execution of a consensus instance, the *CheckTxState* may be updated concurrently with the
+*ExecuteTxState*, as messages may be sent concurrently on the Consensus and Mempool connections.
+At the end of the consensus instance, as described above, Tendermint locks the mempool and flushes
+the mempool connection before calling `Commit`. This ensures that all pending `CheckTx` calls are
+responded to and no new ones can begin.
+
+After the `Commit` call returns, while still holding the mempool lock, `CheckTx` is run again on all
+transactions that remain in the node's local mempool after filtering those included in the block.
+Parameter `Type` in `RequestCheckTx`
+indicates whether an incoming transaction is new (`CheckTxType_New`), or a
+recheck (`CheckTxType_Recheck`).
+
+Finally, after re-checking transactions in the mempool, Tendermint will unlock
+the mempool connection. New transactions are once again able to be processed through `CheckTx`.
+
+Note that `CheckTx` is just a weak filter to keep invalid transactions out of the mempool and,
+utimately, ouf of the blockchain.
+Since the transaction cannot be guaranteed to be checked against the exact same state as it
+will be executed as part of a (potential) decided block, `CheckTx` shouldn't check *everything*
+that affects the transaction's validity, in particular those checks whose validity may depend on
+transaction ordering. `CheckTx` is weak because a Byzantine node need not care about `CheckTx`;
+it can propose a block full of invalid transactions if it wants. The mechanism ABCI++ has
+in place for dealing with such behavior is `ProcessProposal`.
+
+##### Replay Protection
+
+It is possible for old transactions to be sent again to the Application. This is typically
+undesirable for all transactions, except for a generally small subset of them which are idempotent.
+
+The mempool has a mechanism to prevent duplicated transactions from being processed.
+This mechanism is nevertheless best-effort (currently based on the indexer)
+and does not provide any guarantee of non duplication.
+It is thus up to the Application to implement an application-specific
+replay protection mechanism with strong guarantees as part of the logic in `CheckTx`.
+
+#### Info/Query Connection
+
+The Info (or Query) Connection should maintain a `QueryState`. This connection has two
+purposes: 1) having the application answer the queries Tenderissued receives from users
+(see section [Query](#query)),
+and 2) synchronizing Tendermint and the Application at start up time (see
+[Crash Recovery](#crash-recovery))
+or after state sync (see [State Sync](#state-sync)).
+
+`QueryState` is a read-only copy of *ExecuteTxState* as it was after the last
+`Commit`, i.e.
+after the full block has been processed and the state committed to disk.
+
+#### Snapshot Connection
+
+The Snapshot Connection is used to serve state sync snapshots for other nodes
+and/or restore state sync snapshots to a local node being bootstrapped.
+Snapshop management is optional: an Application may choose not to implement it.
+
+For more information, see Section [State Sync](#state-sync).
+
+### Transaction Results
+
+The Application is expected to return a list of
+[`ExecTxResult`](./abci%2B%2B_methods_002_draft.md#exectxresult) in
+[`ResponseFinalizeBlock`](./abci%2B%2B_methods_002_draft.md#finalizeblock). The list of transaction
+results must respect the same order as the list of transactions delivered via
+[`RequestFinalizeBlock`](./abci%2B%2B_methods_002_draft.md#finalizeblock).
+This section discusses the fields inside this structure, along with the fields in
+[`ResponseCheckTx`](./abci%2B%2B_methods_002_draft.md#checktx),
+whose semantics are similar.
+
+The `Info` and `Log` fields are
+non-deterministic values for debugging/convenience purposes. Tendermint logs them but they
+are otherwise ignored.
+
+#### Gas
+
+Ethereum introduced the notion of *gas* as an abstract representation of the
+cost of the resources consumed by nodes when processing a transaction. Every operation in the
+Ethereum Virtual Machine uses some amount of gas.
+Gas has a market-variable price based on which miners can accept or reject to execute a
+particular operation.
+
+Users propose a maximum amount of gas for their transaction; if the transaction uses less, they get
+the difference credited back. Tendermint adopts a similar abstraction,
+though uses it only optionally and weakly, allowing applications to define
+their own sense of the cost of execution.
+
+In Tendermint, the [ConsensusParams.Block.MaxGas](#consensus-parameters) limits the amount of
+total gas that can be used by all transactions in a block.
+The default value is `-1`, which means the block gas limit is not enforced, or that the concept of
+gas is meaningless.
+
+Responses contain a `GasWanted` and `GasUsed` field. The former is the maximum
+amount of gas the sender of a transaction is willing to use, and the latter is how much it actually
+used. Applications should enforce that `GasUsed <= GasWanted` — i.e. transaction execution
+or validation should fail before it can use more resources than it requested.
+
+When `MaxGas > -1`, Tendermint enforces the following rules:
+
+* `GasWanted <= MaxGas` for every transaction in the mempool
+* `(sum of GasWanted in a block) <= MaxGas` when proposing a block
+
+If `MaxGas == -1`, no rules about gas are enforced.
+
+In v0.35.x and earlier versions, Tendermint does not enforce anything about Gas in consensus,
+only in the mempool.
+This means it does not guarantee that committed blocks satisfy these rules.
+It is the application's responsibility to return non-zero response codes when gas limits are exceeded
+when executing the transactions of a block.
+Since the introduction of `PrepareProposal` and `ProcessProposal` in v.0.36.x, it is now possible
+for the Application to enforce that all blocks proposed (and voted for) in consensus — and thus all
+blocks decided — respect the `MaxGas` limits described above.
+
+Since the Application should enforce that `GasUsed <= GasWanted` when executing a transaction, and
+it can use `PrepareProposal` and `ProcessProposal` to enforce that `(sum of GasWanted in a block) <= MaxGas`
+in all proposed or prevoted blocks,
+we have:
+
+* `(sum of GasUsed in a block) <= MaxGas` for every block
+
+The `GasUsed` field is ignored by Tendermint.
+
+#### Specifics of `ResponseCheckTx`
+
+If `Code != 0`, it will be rejected from the mempool and hence
+not broadcasted to other peers and not included in a proposal block.
+
+`Data` contains the result of the `CheckTx` transaction execution, if any. It does not need to be
+deterministic since, given a transaction, nodes' Applications
+might have a different *CheckTxState* values when they receive it and check their validity
+via `CheckTx`.
+Tendermint ignores this value in `ResponseCheckTx`.
+
+`Events` include any events for the execution, though since the transaction has not
+been committed yet, they are effectively ignored by Tendermint.
+
+From v0.35.x on, there is a `Priority` field in `ResponseCheckTx` that can be
+used to explicitly prioritize transactions in the mempool for inclusion in a block
+proposal.
+
+#### Specifics of `ExecTxResult`
+
+`FinalizeBlock` is the workhorse of the blockchain. Tendermint delivers the decided block,
+including the list of all its transactions synchronously to the Application.
+The block delivered (and thus the transaction order) is the same at all correct nodes as guaranteed
+by the Agreement property of Tendermint consensus.
+
+In same block execution mode, field `LastResultsHash` in the block header refers to the results
+of all transactions stored in that block. Therefore,
+`PrepareProposal` must return `ExecTxResult` so that it can
+be used to build the block to be proposed in the current height.
+
+The `Data` field in `ExecTxResult` contains an array of bytes with the transaction result.
+It must be deterministic (i.e., the same value must be returned at all nodes), but it can contain arbitrary
+data. Likewise, the value of `Code` must be deterministic.
+If `Code != 0`, the transaction will be marked invalid,
+though it is still included in the block. Invalid transaction are not indexed, as they are
+considered analogous to those that failed `CheckTx`.
+
+Both the `Code` and `Data` are included in a structure that is hashed into the
+`LastResultsHash` of the block header in the next height (next block execution mode), or the
+header of the block to propose in the current height (same block execution mode, `ExecTxResult` as
+part of `PrepareProposal`).
+
+`Events` include any events for the execution, which Tendermint will use to index
+the transaction by. This allows transactions to be queried according to what
+events took place during their execution.
+
+### Updating the Validator Set
+
+The application may set the validator set during
+[`InitChain`](./abci%2B%2B_methods_002_draft.md#initchain), and may update it during
+[`FinalizeBlock`](./abci%2B%2B_methods_002_draft.md#finalizeblock)
+(next block execution mode) or
+[`PrepareProposal`](./abci%2B%2B_methods_002_draft.md#prepareproposal)/[`ProcessProposal`](./abci%2B%2B_methods_002_draft.md#processproposal)
+(same block execution mode). In all cases, a structure of type
+[`ValidatorUpdate`](./abci%2B%2B_methods_002_draft.md#validatorupdate) is returned.
+
+The `InitChain` method, used to initialize the Application, can return a list of validators.
+If the list is empty, Tendermint will use the validators loaded from the genesis
+file.
+If the list returned by `InitChain` is not empty, Tendermint will use its contents as the validator set.
+This way the application can set the initial validator set for the
+blockchain.
+
+Applications must ensure that a single set of validator updates does not contain duplicates, i.e.
+a given public key can only appear once within a given update. If an update includes
+duplicates, the block execution will fail irrecoverably.
+
+Structure `ValidatorUpdate` contains a public key, which is used to identify the validator:
+The public key currently supports three types:
+
+* `ed25519`
+* `secp256k1`
+* `sr25519`
+
+Structure `ValidatorUpdate` also contains an `ìnt64` field denoting the validator's new power.
+Applications must ensure that
+`ValidatorUpdate` structures abide by the following rules:
+
+* power must be non-negative
+* if power is set to 0, the validator must be in the validator set; it will be removed from the set
+* if power is greater than 0:
+ * if the validator is not in the validator set, it will be added to the
+ set with the given power
+ * if the validator is in the validator set, its power will be adjusted to the given power
+* the total power of the new validator set must not exceed `MaxTotalVotingPower`, where
+ `MaxTotalVotingPower = MaxInt64 / 8`
+
+Note the updates returned after processing the block at height `H` will only take effect
+at block `H+2` (see Section [Methods](./abci%2B%2B_methods_002_draft.md)).
+
+### Consensus Parameters
+
+`ConsensusParams` are global parameters that apply to all validators in a blockchain.
+They enforce certain limits in the blockchain, like the maximum size
+of blocks, amount of gas used in a block, and the maximum acceptable age of
+evidence. They can be set in
+[`InitChain`](./abci%2B%2B_methods_002_draft.md#initchain), and updated in
+[`FinalizeBlock`](./abci%2B%2B_methods_002_draft.md#finalizeblock)
+(next block execution mode) or
+[`PrepareProposal`](./abci%2B%2B_methods_002_draft.md#prepareproposal)/[`ProcessProposal`](./abci%2B%2B_methods_002_draft.md#processproposal)
+(same block execution model).
+These parameters are deterministically set and/or updated by the Application, so
+all full nodes have the same value at a given height.
+
+#### List of Parameters
+
+These are the current consensus parameters (as of v0.36.x):
+
+1. [BlockParams.MaxBytes](#blockparamsmaxbytes)
+2. [BlockParams.MaxGas](#blockparamsmaxgas)
+3. [EvidenceParams.MaxAgeDuration](#evidenceparamsmaxageduration)
+4. [EvidenceParams.MaxAgeNumBlocks](#evidenceparamsmaxagenumblocks)
+5. [EvidenceParams.MaxBytes](#evidenceparamsmaxbytes)
+6. [SynchronyParams.MessageDelay](#synchronyparamsmessagedelay)
+7. [SynchronyParams.Precision](#synchronyparamsprecision)
+8. [TimeoutParams.Propose](#timeoutparamspropose)
+9. [TimeoutParams.ProposeDelta](#timeoutparamsproposedelta)
+10. [TimeoutParams.Vote](#timeoutparamsvote)
+11. [TimeoutParams.VoteDelta](#timeoutparamsvotedelta)
+12. [TimeoutParams.Commit](#timeoutparamscommit)
+13. [TimeoutParams.BypassCommitTimeout](#timeoutparamsbypasscommittimeout)
+
+##### BlockParams.MaxBytes
+
+The maximum size of a complete Protobuf encoded block.
+This is enforced by Tendermint consensus.
+
+This implies a maximum transaction size that is this `MaxBytes`, less the expected size of
+the header, the validator set, and any included evidence in the block.
+
+Must have `0 < MaxBytes < 100 MB`.
+
+##### BlockParams.MaxGas
+
+The maximum of the sum of `GasWanted` that will be allowed in a proposed block.
+This is *not* enforced by Tendermint consensus.
+It is left to the Application to enforce (ie. if transactions are included past the
+limit, they should return non-zero codes). It is used by Tendermint to limit the
+transactions included in a proposed block.
+
+Must have `MaxGas >= -1`.
+If `MaxGas == -1`, no limit is enforced.
+
+##### EvidenceParams.MaxAgeDuration
+
+This is the maximum age of evidence in time units.
+This is enforced by Tendermint consensus.
+
+If a block includes evidence older than this (AND the evidence was created more
+than `MaxAgeNumBlocks` ago), the block will be rejected (validators won't vote
+for it).
+
+Must have `MaxAgeDuration > 0`.
+
+##### EvidenceParams.MaxAgeNumBlocks
+
+This is the maximum age of evidence in blocks.
+This is enforced by Tendermint consensus.
+
+If a block includes evidence older than this (AND the evidence was created more
+than `MaxAgeDuration` ago), the block will be rejected (validators won't vote
+for it).
+
+Must have `MaxAgeNumBlocks > 0`.
+
+##### EvidenceParams.MaxBytes
+
+This is the maximum size of total evidence in bytes that can be committed to a
+single block. It should fall comfortably under the max block bytes.
+
+Its value must not exceed the size of
+a block minus its overhead ( ~ `BlockParams.MaxBytes`).
+
+Must have `MaxBytes > 0`.
+
+##### SynchronyParams.MessageDelay
+
+This sets a bound on how long a proposal message may take to reach all
+validators on a network and still be considered valid.
+
+This parameter is part of the
+[proposer-based timestamps](../consensus/proposer-based-timestamp)
+(PBTS) algorithm.
+
+##### SynchronyParams.Precision
+
+This sets a bound on how skewed a proposer's clock may be from any validator
+on the network while still producing valid proposals.
+
+This parameter is part of the
+[proposer-based timestamps](../consensus/proposer-based-timestamp)
+(PBTS) algorithm.
+
+##### TimeoutParams.Propose
+
+Timeout in ms of the propose step of the Tendermint consensus algorithm.
+This value is the initial timeout at every height (round 0).
+
+The value in subsequent rounds is modified by parameter `ProposeDelta`.
+When a new height is started, the `Propose` timeout value is reset to this
+parameter.
+
+If a node waiting for a proposal message does not receive one matching its
+current height and round before this timeout, the node will issue a
+`nil` prevote for the round and advance to the next step.
+
+##### TimeoutParams.ProposeDelta
+
+Increment in ms to be added to the `Propose` timeout every time the Tendermint
+consensus algorithm advances one round in a given height.
+
+When a new height is started, the `Propose` timeout value is reset.
+
+##### TimeoutParams.Vote
+
+Timeout in ms of the prevote and precommit steps of the Tendermint consensus
+algorithm.
+This value is the initial timeout at every height (round 0).
+
+The value in subsequent rounds is modified by parameter `VoteDelta`.
+When a new height is started, the `Vote` timeout value is reset to this
+parameter.
+
+The `Vote` timeout does not begin until a quorum of votes has been received.
+Once a quorum of votes has been seen and this timeout elapses, Tendermint will
+procced to the next step of the consensus algorithm. If Tendermint receives
+all of the remaining votes before the end of the timeout, it will proceed
+to the next step immediately.
+
+##### TimeoutParams.VoteDelta
+
+Increment in ms to be added to the `Vote` timeout every time the Tendermint
+consensus algorithm advances one round in a given height.
+
+When a new height is started, the `Vote` timeout value is reset.
+
+##### TimeoutParams.Commit
+
+This configures how long Tendermint will wait after receiving a quorum of
+precommits before beginning consensus for the next height. This can be
+used to allow slow precommits to arrive for inclusion in the next height
+before progressing.
+
+##### TimeoutParams.BypassCommitTimeout
+
+This configures the node to proceed immediately to the next height once the
+node has received all precommits for a block, forgoing the remaining commit timeout.
+Setting this parameter to `false` (the default) causes Tendermint to wait
+for the full commit timeout configured in `TimeoutParams.Commit`.
+
+#### Updating Consensus Parameters
+
+The application may set the `ConsensusParams` during
+[`InitChain`](./abci%2B%2B_methods_002_draft.md#initchain),
+and update them during
+[`FinalizeBlock`](./abci%2B%2B_methods_002_draft.md#finalizeblock)
+(next block execution mode) or
+[`PrepareProposal`](./abci%2B%2B_methods_002_draft.md#prepareproposal)/[`ProcessProposal`](./abci%2B%2B_methods_002_draft.md#processproposal)
+(same block execution mode).
+If the `ConsensusParams` is empty, it will be ignored. Each field
+that is not empty will be applied in full. For instance, if updating the
+`Block.MaxBytes`, applications must also set the other `Block` fields (like
+`Block.MaxGas`), even if they are unchanged, as they will otherwise cause the
+value to be updated to the default.
+
+##### `InitChain`
+
+`ResponseInitChain` includes a `ConsensusParams` parameter.
+If `ConsensusParams` is `nil`, Tendermint will use the params loaded in the genesis
+file. If `ConsensusParams` is not `nil`, Tendermint will use it.
+This way the application can determine the initial consensus parameters for the
+blockchain.
+
+##### `FinalizeBlock`, `PrepareProposal`/`ProcessProposal`
+
+In next block execution mode, `ResponseFinalizeBlock` accepts a `ConsensusParams` parameter.
+If `ConsensusParams` is `nil`, Tendermint will do nothing.
+If `ConsensusParams` is not `nil`, Tendermint will use it.
+This way the application can update the consensus parameters over time.
+
+Likewise, in same block execution mode, `PrepareProposal` and `ProcessProposal` include
+a `ConsensusParams` parameter. `PrepareProposal` may return a `ConsensusParams` to update
+the consensus parameters in the block that is about to be proposed. If it returns `nil`
+the consensus parameters will not be updated. `ProcessProposal` also accepts a
+`ConsensusParams` parameter, which Tendermint will use it to calculate the corresponding
+hashes and sanity-check them against those of the block that triggered `ProcessProposal`
+at the first place.
+
+Note the updates returned in block `H` will take effect right away for block
+`H+1` (both in next block and same block execution mode).
+
+### `Query`
+
+`Query` is a generic method with lots of flexibility to enable diverse sets
+of queries on application state. Tendermint makes use of `Query` to filter new peers
+based on ID and IP, and exposes `Query` to the user over RPC.
+
+Note that calls to `Query` are not replicated across nodes, but rather query the
+local node's state - hence they may return stale reads. For reads that require
+consensus, use a transaction.
+
+The most important use of `Query` is to return Merkle proofs of the application state at some height
+that can be used for efficient application-specific light-clients.
+
+Note Tendermint has technically no requirements from the `Query`
+message for normal operation - that is, the ABCI app developer need not implement
+Query functionality if they do not wish to.
+
+#### Query Proofs
+
+The Tendermint block header includes a number of hashes, each providing an
+anchor for some type of proof about the blockchain. The `ValidatorsHash` enables
+quick verification of the validator set, the `DataHash` gives quick
+verification of the transactions included in the block.
+
+The `AppHash` is unique in that it is application specific, and allows for
+application-specific Merkle proofs about the state of the application.
+While some applications keep all relevant state in the transactions themselves
+(like Bitcoin and its UTXOs), others maintain a separated state that is
+computed deterministically *from* transactions, but is not contained directly in
+the transactions themselves (like Ethereum contracts and accounts).
+For such applications, the `AppHash` provides a much more efficient way to verify light-client proofs.
+
+ABCI applications can take advantage of more efficient light-client proofs for
+their state as follows:
+
+* in next block executon mode, return the Merkle root of the deterministic application state in
+ `ResponseCommit.Data`. This Merkle root will be included as the `AppHash` in the next block.
+* in same block execution mode, return the Merkle root of the deterministic application state
+ in `ResponsePrepareProposal.AppHash`. This Merkle root will be included as the `AppHash` in
+ the block that is about to be proposed.
+* return efficient Merkle proofs about that application state in `ResponseQuery.Proof`
+ that can be verified using the `AppHash` of the corresponding block.
+
+For instance, this allows an application's light-client to verify proofs of
+absence in the application state, something which is much less efficient to do using the block hash.
+
+Some applications (eg. Ethereum, Cosmos-SDK) have multiple "levels" of Merkle trees,
+where the leaves of one tree are the root hashes of others. To support this, and
+the general variability in Merkle proofs, the `ResponseQuery.Proof` has some minimal structure:
+
+```protobuf
+message ProofOps {
+ repeated ProofOp ops = 1
+}
+
+message ProofOp {
+ string type = 1;
+ bytes key = 2;
+ bytes data = 3;
+}
+```
+
+Each `ProofOp` contains a proof for a single key in a single Merkle tree, of the specified `type`.
+This allows ABCI to support many different kinds of Merkle trees, encoding
+formats, and proofs (eg. of presence and absence) just by varying the `type`.
+The `data` contains the actual encoded proof, encoded according to the `type`.
+When verifying the full proof, the root hash for one ProofOp is the value being
+verified for the next ProofOp in the list. The root hash of the final ProofOp in
+the list should match the `AppHash` being verified against.
+
+#### Peer Filtering
+
+When Tendermint connects to a peer, it sends two queries to the ABCI application
+using the following paths, with no additional data:
+
+* `/p2p/filter/addr/`, where `` denote the IP address and
+ the port of the connection
+* `p2p/filter/id/`, where `` is the peer node ID (ie. the
+ pubkey.Address() for the peer's PubKey)
+
+If either of these queries return a non-zero ABCI code, Tendermint will refuse
+to connect to the peer.
+
+#### Paths
+
+Queries are directed at paths, and may optionally include additional data.
+
+The expectation is for there to be some number of high level paths
+differentiating concerns, like `/p2p`, `/store`, and `/app`. Currently,
+Tendermint only uses `/p2p`, for filtering peers. For more advanced use, see the
+implementation of
+[Query in the Cosmos-SDK](https://github.com/cosmos/cosmos-sdk/blob/v0.23.1/baseapp/baseapp.go#L333).
+
+### Crash Recovery
+
+On startup, Tendermint calls the `Info` method on the Info Connection to get the latest
+committed state of the app. The app MUST return information consistent with the
+last block it succesfully completed Commit for.
+
+If the app succesfully committed block H, then `last_block_height = H` and `last_block_app_hash = `. If the app
+failed during the Commit of block H, then `last_block_height = H-1` and
+`last_block_app_hash = `.
+
+We now distinguish three heights, and describe how Tendermint syncs itself with
+the app.
+
+```md
+storeBlockHeight = height of the last block Tendermint saw a commit for
+stateBlockHeight = height of the last block for which Tendermint completed all
+ block processing and saved all ABCI results to disk
+appBlockHeight = height of the last block for which ABCI app succesfully
+ completed Commit
+
+```
+
+Note we always have `storeBlockHeight >= stateBlockHeight` and `storeBlockHeight >= appBlockHeight`
+Note also Tendermint never calls Commit on an ABCI app twice for the same height.
+
+The procedure is as follows.
+
+First, some simple start conditions:
+
+If `appBlockHeight == 0`, then call InitChain.
+
+If `storeBlockHeight == 0`, we're done.
+
+Now, some sanity checks:
+
+If `storeBlockHeight < appBlockHeight`, error
+If `storeBlockHeight < stateBlockHeight`, panic
+If `storeBlockHeight > stateBlockHeight+1`, panic
+
+Now, the meat:
+
+If `storeBlockHeight == stateBlockHeight && appBlockHeight < storeBlockHeight`,
+replay all blocks in full from `appBlockHeight` to `storeBlockHeight`.
+This happens if we completed processing the block, but the app forgot its height.
+
+If `storeBlockHeight == stateBlockHeight && appBlockHeight == storeBlockHeight`, we're done.
+This happens if we crashed at an opportune spot.
+
+If `storeBlockHeight == stateBlockHeight+1`
+This happens if we started processing the block but didn't finish.
+
+If `appBlockHeight < stateBlockHeight`
+ replay all blocks in full from `appBlockHeight` to `storeBlockHeight-1`,
+ and replay the block at `storeBlockHeight` using the WAL.
+This happens if the app forgot the last block it committed.
+
+If `appBlockHeight == stateBlockHeight`,
+ replay the last block (storeBlockHeight) in full.
+This happens if we crashed before the app finished Commit
+
+If `appBlockHeight == storeBlockHeight`
+ update the state using the saved ABCI responses but dont run the block against the real app.
+This happens if we crashed after the app finished Commit but before Tendermint saved the state.
+
+### State Sync
+
+A new node joining the network can simply join consensus at the genesis height and replay all
+historical blocks until it is caught up. However, for large chains this can take a significant
+amount of time, often on the order of days or weeks.
+
+State sync is an alternative mechanism for bootstrapping a new node, where it fetches a snapshot
+of the state machine at a given height and restores it. Depending on the application, this can
+be several orders of magnitude faster than replaying blocks.
+
+Note that state sync does not currently backfill historical blocks, so the node will have a
+truncated block history - users are advised to consider the broader network implications of this in
+terms of block availability and auditability. This functionality may be added in the future.
+
+For details on the specific ABCI calls and types, see the
+[methods](abci%2B%2B_methods_002_draft.md) section.
+
+#### Taking Snapshots
+
+Applications that want to support state syncing must take state snapshots at regular intervals. How
+this is accomplished is entirely up to the application. A snapshot consists of some metadata and
+a set of binary chunks in an arbitrary format:
+
+* `Height (uint64)`: The height at which the snapshot is taken. It must be taken after the given
+ height has been committed, and must not contain data from any later heights.
+
+* `Format (uint32)`: An arbitrary snapshot format identifier. This can be used to version snapshot
+ formats, e.g. to switch from Protobuf to MessagePack for serialization. The application can use
+ this when restoring to choose whether to accept or reject a snapshot.
+
+* `Chunks (uint32)`: The number of chunks in the snapshot. Each chunk contains arbitrary binary
+ data, and should be less than 16 MB; 10 MB is a good starting point.
+
+* `Hash ([]byte)`: An arbitrary hash of the snapshot. This is used to check whether a snapshot is
+ the same across nodes when downloading chunks.
+
+* `Metadata ([]byte)`: Arbitrary snapshot metadata, e.g. chunk hashes for verification or any other
+ necessary info.
+
+For a snapshot to be considered the same across nodes, all of these fields must be identical. When
+sent across the network, snapshot metadata messages are limited to 4 MB.
+
+When a new node is running state sync and discovering snapshots, Tendermint will query an existing
+application via the ABCI `ListSnapshots` method to discover available snapshots, and load binary
+snapshot chunks via `LoadSnapshotChunk`. The application is free to choose how to implement this
+and which formats to use, but must provide the following guarantees:
+
+* **Consistent:** A snapshot must be taken at a single isolated height, unaffected by
+ concurrent writes. This can be accomplished by using a data store that supports ACID
+ transactions with snapshot isolation.
+
+* **Asynchronous:** Taking a snapshot can be time-consuming, so it must not halt chain progress,
+ for example by running in a separate thread.
+
+* **Deterministic:** A snapshot taken at the same height in the same format must be identical
+ (at the byte level) across nodes, including all metadata. This ensures good availability of
+ chunks, and that they fit together across nodes.
+
+A very basic approach might be to use a datastore with MVCC transactions (such as RocksDB),
+start a transaction immediately after block commit, and spawn a new thread which is passed the
+transaction handle. This thread can then export all data items, serialize them using e.g.
+Protobuf, hash the byte stream, split it into chunks, and store the chunks in the file system
+along with some metadata - all while the blockchain is applying new blocks in parallel.
+
+A more advanced approach might include incremental verification of individual chunks against the
+chain app hash, parallel or batched exports, compression, and so on.
+
+Old snapshots should be removed after some time - generally only the last two snapshots are needed
+(to prevent the last one from being removed while a node is restoring it).
+
+#### Bootstrapping a Node
+
+An empty node can be state synced by setting the configuration option `statesync.enabled =
+true`. The node also needs the chain genesis file for basic chain info, and configuration for
+light client verification of the restored snapshot: a set of Tendermint RPC servers, and a
+trusted header hash and corresponding height from a trusted source, via the `statesync`
+configuration section.
+
+Once started, the node will connect to the P2P network and begin discovering snapshots. These
+will be offered to the local application via the `OfferSnapshot` ABCI method. Once a snapshot
+is accepted Tendermint will fetch and apply the snapshot chunks. After all chunks have been
+successfully applied, Tendermint verifies the app's `AppHash` against the chain using the light
+client, then switches the node to normal consensus operation.
+
+##### Snapshot Discovery
+
+When the empty node joins the P2P network, it asks all peers to report snapshots via the
+`ListSnapshots` ABCI call (limited to 10 per node). After some time, the node picks the most
+suitable snapshot (generally prioritized by height, format, and number of peers), and offers it
+to the application via `OfferSnapshot`. The application can choose a number of responses,
+including accepting or rejecting it, rejecting the offered format, rejecting the peer who sent
+it, and so on. Tendermint will keep discovering and offering snapshots until one is accepted or
+the application aborts.
+
+##### Snapshot Restoration
+
+Once a snapshot has been accepted via `OfferSnapshot`, Tendermint begins downloading chunks from
+any peers that have the same snapshot (i.e. that have identical metadata fields). Chunks are
+spooled in a temporary directory, and then given to the application in sequential order via
+`ApplySnapshotChunk` until all chunks have been accepted.
+
+The method for restoring snapshot chunks is entirely up to the application.
+
+During restoration, the application can respond to `ApplySnapshotChunk` with instructions for how
+to continue. This will typically be to accept the chunk and await the next one, but it can also
+ask for chunks to be refetched (either the current one or any number of previous ones), P2P peers
+to be banned, snapshots to be rejected or retried, and a number of other responses - see the ABCI
+reference for details.
+
+If Tendermint fails to fetch a chunk after some time, it will reject the snapshot and try a
+different one via `OfferSnapshot` - the application can choose whether it wants to support
+restarting restoration, or simply abort with an error.
+
+##### Snapshot Verification
+
+Once all chunks have been accepted, Tendermint issues an `Info` ABCI call to retrieve the
+`LastBlockAppHash`. This is compared with the trusted app hash from the chain, retrieved and
+verified using the light client. Tendermint also checks that `LastBlockHeight` corresponds to the
+height of the snapshot.
+
+This verification ensures that an application is valid before joining the network. However, the
+snapshot restoration may take a long time to complete, so applications may want to employ additional
+verification during the restore to detect failures early. This might e.g. include incremental
+verification of each chunk against the app hash (using bundled Merkle proofs), checksums to
+protect against data corruption by the disk or network, and so on. However, it is important to
+note that the only trusted information available is the app hash, and all other snapshot metadata
+can be spoofed by adversaries.
+
+Apps may also want to consider state sync denial-of-service vectors, where adversaries provide
+invalid or harmful snapshots to prevent nodes from joining the network. The application can
+counteract this by asking Tendermint to ban peers. As a last resort, node operators can use
+P2P configuration options to whitelist a set of trusted peers that can provide valid snapshots.
+
+##### Transition to Consensus
+
+Once the snapshots have all been restored, Tendermint gathers additional information necessary for
+bootstrapping the node (e.g. chain ID, consensus parameters, validator sets, and block headers)
+from the genesis file and light client RPC servers. It also calls `Info` to verify the following:
+
+* that the app hash from the snapshot it has delivered to the Application matches the apphash
+ stored in the next height's block (in next block execution), or the current block's height
+ (same block execution)
+* that the version that the Application returns in `ResponseInfo` matches the version in the
+ current height's block header
+
+Once the state machine has been restored and Tendermint has gathered this additional
+information, it transitions to block sync (if enabled) to fetch any remaining blocks up the chain
+head, and then transitions to regular consensus operation. At this point the node operates like
+any other node, apart from having a truncated block history at the height of the restored snapshot.
diff --git a/spec/abci++/abci++_methods_002_draft.md b/spec/abci++/abci++_methods_002_draft.md
index d1782bbdc..4113a0c58 100644
--- a/spec/abci++/abci++_methods_002_draft.md
+++ b/spec/abci++/abci++_methods_002_draft.md
@@ -80,13 +80,15 @@ title: Methods
* **Usage**:
* Called once upon genesis.
- * If ResponseInitChain.Validators is empty, the initial validator set will be the RequestInitChain.Validators
- * If ResponseInitChain.Validators is not empty, it will be the initial
- validator set (regardless of what is in RequestInitChain.Validators).
+ * If `ResponseInitChain.Validators` is empty, the initial validator set will be the `RequestInitChain.Validators`
+ * If `ResponseInitChain.Validators` is not empty, it will be the initial
+ validator set (regardless of what is in `RequestInitChain.Validators`).
* This allows the app to decide if it wants to accept the initial validator
- set proposed by tendermint (ie. in the genesis file), or if it wants to use
- a different one (perhaps computed based on some application specific
- information in the genesis file).
+ set proposed by tendermint (ie. in the genesis file), or if it wants to use
+ a different one (perhaps computed based on some application specific
+ information in the genesis file).
+ * Both `ResponseInitChain.Validators` and `ResponseInitChain.Validators` are [ValidatorUpdate](#validatorupdate) structs.
+ So, technically, they both are _updating_ the set of validators from the empty set.
### Query
@@ -302,7 +304,7 @@ title: Methods
|-------------------------|--------------------------------------------------|---------------------------------------------------------------------------------------------|--------------|
| tx_records | repeated [TxRecord](#txrecord) | Possibly modified list of transactions that have been picked as part of the proposed block. | 2 |
| app_hash | bytes | The Merkle root hash of the application state. | 3 |
- | tx_results | repeated [ExecTxResult](#txresult) | List of structures containing the data resulting from executing the transactions | 4 |
+ | tx_results | repeated [ExecTxResult](#exectxresult) | List of structures containing the data resulting from executing the transactions | 4 |
| validator_updates | repeated [ValidatorUpdate](#validatorupdate) | Changes to validator set (set voting power to 0 to remove). | 5 |
| consensus_param_updates | [ConsensusParams](#consensusparams) | Changes to consensus-critical gas, size, and other parameters. | 6 |
@@ -414,7 +416,7 @@ Note that, if _p_ has a non-`nil` _validValue_, Tendermint will use it as propos
|-------------------------|--------------------------------------------------|-----------------------------------------------------------------------------------|--------------|
| status | [ProposalStatus](#proposalstatus) | `enum` that signals if the application finds the proposal valid. | 1 |
| app_hash | bytes | The Merkle root hash of the application state. | 2 |
- | tx_results | repeated [ExecTxResult](#txresult) | List of structures containing the data resulting from executing the transactions. | 3 |
+ | tx_results | repeated [ExecTxResult](#exectxresult) | List of structures containing the data resulting from executing the transactions. | 3 |
| validator_updates | repeated [ValidatorUpdate](#validatorupdate) | Changes to validator set (set voting power to 0 to remove). | 4 |
| consensus_param_updates | [ConsensusParams](#consensusparams) | Changes to consensus-critical gas, size, and other parameters. | 5 |
@@ -582,7 +584,7 @@ from this condition, but not sure), and _p_ receives a Precommit message for rou
| Name | Type | Description | Field Number |
|-------------------------|-------------------------------------------------------------|----------------------------------------------------------------------------------|--------------|
| events | repeated [Event](abci++_basic_concepts_002_draft.md#events) | Type & Key-Value events for indexing | 1 |
- | tx_results | repeated [ExecTxResult](#txresult) | List of structures containing the data resulting from executing the transactions | 2 |
+ | tx_results | repeated [ExecTxResult](#exectxresult) | List of structures containing the data resulting from executing the transactions | 2 |
| validator_updates | repeated [ValidatorUpdate](#validatorupdate) | Changes to validator set (set voting power to 0 to remove). | 3 |
| consensus_param_updates | [ConsensusParams](#consensusparams) | Changes to consensus-critical gas, size, and other parameters. | 4 |
| app_hash | bytes | The Merkle root hash of the application state. | 5 |
From 850ae93a90a507b6fdaf05c796d69de5126ace98 Mon Sep 17 00:00:00 2001
From: Sergio Mena
Date: Thu, 19 May 2022 10:35:36 +0200
Subject: [PATCH 34/39] Adapted `client-server.md` from ABCI directory (#8510)
* Copied over 'client server' section from ABCI spec
* Adapted the ABCI text in 'Client and Server' section
* Minor changes to README
* Removed TODO from Readme
* Update spec/abci++/abci++_client_server_002_draft.md
Co-authored-by: Daniel
* Update spec/abci++/abci++_client_server_002_draft.md
Co-authored-by: Daniel
* Update spec/abci++/abci++_client_server_002_draft.md
Co-authored-by: Daniel
* Update spec/abci++/abci++_client_server_002_draft.md
Co-authored-by: Daniel
* Update spec/abci++/abci++_client_server_002_draft.md
Co-authored-by: Daniel
* Update spec/abci++/abci++_client_server_002_draft.md
Co-authored-by: Daniel
* Addressed comments
* Moved GRPC link out of the Tendermint-specific occurrence
* Fixed merge
Co-authored-by: Daniel
---
spec/abci++/README.md | 10 +-
spec/abci++/abci++_client_server_002_draft.md | 102 ++++++++++++++++++
2 files changed, 105 insertions(+), 7 deletions(-)
create mode 100644 spec/abci++/abci++_client_server_002_draft.md
diff --git a/spec/abci++/README.md b/spec/abci++/README.md
index 0f7a87c4a..a22babfee 100644
--- a/spec/abci++/README.md
+++ b/spec/abci++/README.md
@@ -25,19 +25,15 @@ This allows Tendermint to run with applications written in many programming lang
This specification is split as follows:
-- [Overview and basic concepts](./abci++_basic_concepts_002_draft.md) - interface's overview and concepts needed to understand other parts of this specification.
+- [Overview and basic concepts](./abci++_basic_concepts_002_draft.md) - interface's overview and concepts
+ needed to understand other parts of this specification.
- [Methods](./abci++_methods_002_draft.md) - complete details on all ABCI++ methods
and message types.
- [Requirements for the Application](./abci++_app_requirements_002_draft.md) - formal requirements
on the Application's logic to ensure Tendermint properties such as liveness. These requirements define what
- Tendermint expects from the Application.
+ Tendermint expects from the Application; second part on managing ABCI application state and related topics.
- [Tendermint's expected behavior](./abci++_tmint_expected_behavior_002_draft.md) - specification of
how the different ABCI++ methods may be called by Tendermint. This explains what the Application
is to expect from Tendermint.
-
->**TODO** Re-read these and remove redundant info
-
-- [Applications](../abci/apps.md) - how to manage ABCI application state and other
- details about building ABCI applications
- [Client and Server](../abci/client-server.md) - for those looking to implement their
own ABCI application servers
diff --git a/spec/abci++/abci++_client_server_002_draft.md b/spec/abci++/abci++_client_server_002_draft.md
new file mode 100644
index 000000000..f26ee8cd5
--- /dev/null
+++ b/spec/abci++/abci++_client_server_002_draft.md
@@ -0,0 +1,102 @@
+---
+order: 5
+title: Client and Server
+---
+
+# Client and Server
+
+This section is for those looking to implement their own ABCI Server, perhaps in
+a new programming language.
+
+You are expected to have read all previous sections of ABCI++ specification, namely
+[Basic Concepts](./abci%2B%2B_basic_concepts_002_draft.md),
+[Methods](./abci%2B%2B_methods_002_draft.md),
+[Application Requirements](./abci%2B%2B_app_requirements_002_draft.md), and
+[Expected Behavior](./abci%2B%2B_tmint_expected_behavior_002_draft.md).
+
+## Message Protocol and Synchrony
+
+The message protocol consists of pairs of requests and responses defined in the
+[protobuf file](../../proto/tendermint/abci/types.proto).
+
+Some messages have no fields, while others may include byte-arrays, strings, integers,
+or custom protobuf types.
+
+For more details on protobuf, see the [documentation](https://developers.google.com/protocol-buffers/docs/overview).
+
+As of v0.36 requests are synchronous. For each of ABCI++'s four connections (see
+[Connections](./abci%2B%2B_app_requirements_002_draft.md)), when Tendermint issues a request to the
+Application, it will wait for the response before continuing execution. As a side effect,
+requests and responses are ordered for each connection, but not necessarily across connections.
+
+## Server Implementations
+
+To use ABCI in your programming language of choice, there must be an ABCI
+server in that language. Tendermint supports four implementations of the ABCI server:
+
+- in Tendermint's repository:
+ - In-process
+ - ABCI-socket
+ - GRPC
+- [tendermint-rs](https://github.com/informalsystems/tendermint-rs)
+- [tower-abci](https://github.com/penumbra-zone/tower-abci)
+
+The implementations in Tendermint's repository can be tested using `abci-cli` by setting
+the `--abci` flag appropriately.
+
+See examples, in various stages of maintenance, in
+[Go](https://github.com/tendermint/tendermint/tree/master/abci/server),
+[JavaScript](https://github.com/tendermint/js-abci),
+[C++](https://github.com/mdyring/cpp-tmsp), and
+[Java](https://github.com/jTendermint/jabci).
+
+### In Process
+
+The simplest implementation uses function calls in Golang.
+This means ABCI applications written in Golang can be linked with Tendermint Core and run as a single binary.
+
+### GRPC
+
+If you are not using Golang,
+but [GRPC](https://grpc.io/) is available in your language, this is the easiest approach,
+though it will have significant performance overhead.
+
+Please check GRPC's documentation to know to set up the Application as an
+ABCI GRPC server.
+
+### Socket
+
+Tendermint's socket-based ABCI interface is an asynchronous,
+raw socket server which provides ordered message passing over unix or tcp.
+Messages are serialized using Protobuf3 and length-prefixed with a [signed Varint](https://developers.google.com/protocol-buffers/docs/encoding?csw=1#signed-integers).
+
+If GRPC is not available in your language, your application requires higher
+performance, or otherwise enjoy programming, you may implement your own
+ABCI server using the Tendermint's socket-based ABCI interface.
+The first step is to auto-generate the relevant data
+types and codec in your language using `protoc`.
+In addition to being proto3 encoded, messages coming over
+the socket are length-prefixed. proto3 doesn't have an
+official length-prefix standard, so we use our own. The first byte in
+the prefix represents the length of the Big Endian encoded length. The
+remaining bytes in the prefix are the Big Endian encoded length.
+
+For example, if the proto3 encoded ABCI message is `0xDEADBEEF` (4
+bytes long), the length-prefixed message is `0x0104DEADBEEF` (`01` byte for encoding the length `04` of the message). If the proto3
+encoded ABCI message is 65535 bytes long, the length-prefixed message
+would start with 0x02FFFF.
+
+Note that this length-prefixing scheme does not apply for GRPC.
+
+Note that your ABCI server must be able to support multiple connections, as
+Tendermint uses four connections.
+
+## Client
+
+There are currently two use-cases for an ABCI client. One is testing
+tools that allow ABCI requests to be sent to the actual application via
+command line. An example of this is `abci-cli`, which accepts CLI commands
+to send corresponding ABCI requests.
+The other is a consensus engine, such as Tendermint Core,
+which makes ABCI requests to the application as prescribed by the consensus
+algorithm used.
From b4bf74ba9c367b5b640f7da14eb40bf2dad24a5e Mon Sep 17 00:00:00 2001
From: Sam Kleinman
Date: Thu, 19 May 2022 18:16:34 +0200
Subject: [PATCH 35/39] abci: serialize semantics of abci client (#8578)
Prior to this change, it was possible that two client calls could enqueue their
requests in the response queue in a different order than they were processed
by the sender goroutine. This violates the requirement that responses must be
delivered in the same order they were enqueued.
To avert this, make the sender goroutine responsible for enqueuing.
Also, remove an unnecessary channel buffer.
---
abci/client/socket_client.go | 13 ++++---------
1 file changed, 4 insertions(+), 9 deletions(-)
diff --git a/abci/client/socket_client.go b/abci/client/socket_client.go
index aa4fdcbe9..8904d557d 100644
--- a/abci/client/socket_client.go
+++ b/abci/client/socket_client.go
@@ -17,12 +17,6 @@ import (
"github.com/tendermint/tendermint/libs/service"
)
-const (
- // reqQueueSize is the max number of queued async requests.
- // (memory: 256MB max assuming 1MB transactions)
- reqQueueSize = 256
-)
-
// This is goroutine-safe, but users should beware that the application in
// general is not meant to be interfaced with concurrent callers.
type socketClient struct {
@@ -48,7 +42,7 @@ var _ Client = (*socketClient)(nil)
func NewSocketClient(logger log.Logger, addr string, mustConnect bool) Client {
cli := &socketClient{
logger: logger,
- reqQueue: make(chan *requestAndResponse, reqQueueSize),
+ reqQueue: make(chan *requestAndResponse),
mustConnect: mustConnect,
addr: addr,
reqSent: list.New(),
@@ -127,6 +121,8 @@ func (cli *socketClient) sendRequestsRoutine(ctx context.Context, conn io.Writer
cli.stopForError(fmt.Errorf("flush buffer: %w", err))
return
}
+
+ cli.trackRequest(reqres)
}
}
}
@@ -158,7 +154,7 @@ func (cli *socketClient) recvResponseRoutine(ctx context.Context, conn io.Reader
}
}
-func (cli *socketClient) willSendReq(reqres *requestAndResponse) {
+func (cli *socketClient) trackRequest(reqres *requestAndResponse) {
cli.mtx.Lock()
defer cli.mtx.Unlock()
@@ -199,7 +195,6 @@ func (cli *socketClient) doRequest(ctx context.Context, req *types.Request) (*ty
}
reqres := makeReqRes(req)
- cli.willSendReq(reqres)
select {
case cli.reqQueue <- reqres:
From 4a9bbe047f8dc762e9020b528a019899c781ddcc Mon Sep 17 00:00:00 2001
From: "M. J. Fromberger"
Date: Thu, 19 May 2022 12:11:57 -0700
Subject: [PATCH 36/39] Fix lock sequencing in socket client request tracking.
(#8581)
* Fix lock sequencing in socket client request tracking.
It is not safe to check base service state (IsRunning) while holding the lock
for the client state. If we do, then during shutdown we may deadlock with the
invocation of the OnStop handler, which the base service executes while holding
the service lock.
* Enqueue pending requests before sending them to the server.
If we don't do this, the server can reply before the request lands in the
queue. That will cause the receiver to terminate early for an unsolicited
response. So enqueue first: This is safe because we're doing it in the same
routine as services the channel, so we won't take another message till we are
safely past that point.
* Document what we did.
* Fix socket paths in tests.
---
abci/client/socket_client.go | 14 +++++++++-----
internal/proxy/client_test.go | 6 +++---
2 files changed, 12 insertions(+), 8 deletions(-)
diff --git a/abci/client/socket_client.go b/abci/client/socket_client.go
index 8904d557d..7dfcf76cc 100644
--- a/abci/client/socket_client.go
+++ b/abci/client/socket_client.go
@@ -112,6 +112,11 @@ func (cli *socketClient) sendRequestsRoutine(ctx context.Context, conn io.Writer
case <-ctx.Done():
return
case reqres := <-cli.reqQueue:
+ // N.B. We must enqueue before sending out the request, otherwise the
+ // server may reply before we do it, and the receiver will fail for an
+ // unsolicited reply.
+ cli.trackRequest(reqres)
+
if err := types.WriteMessage(reqres.Request, bw); err != nil {
cli.stopForError(fmt.Errorf("write to buffer: %w", err))
return
@@ -121,8 +126,6 @@ func (cli *socketClient) sendRequestsRoutine(ctx context.Context, conn io.Writer
cli.stopForError(fmt.Errorf("flush buffer: %w", err))
return
}
-
- cli.trackRequest(reqres)
}
}
}
@@ -155,13 +158,14 @@ func (cli *socketClient) recvResponseRoutine(ctx context.Context, conn io.Reader
}
func (cli *socketClient) trackRequest(reqres *requestAndResponse) {
- cli.mtx.Lock()
- defer cli.mtx.Unlock()
-
+ // N.B. We must NOT hold the client state lock while checking this, or we
+ // may deadlock with shutdown.
if !cli.IsRunning() {
return
}
+ cli.mtx.Lock()
+ defer cli.mtx.Unlock()
cli.reqSent.PushBack(reqres)
}
diff --git a/internal/proxy/client_test.go b/internal/proxy/client_test.go
index 09ac3f2c8..41a34bde7 100644
--- a/internal/proxy/client_test.go
+++ b/internal/proxy/client_test.go
@@ -58,7 +58,7 @@ func (app *appConnTest) Info(ctx context.Context, req *types.RequestInfo) (*type
var SOCKET = "socket"
func TestEcho(t *testing.T) {
- sockPath := fmt.Sprintf("unix:///tmp/echo_%v.sock", tmrand.Str(6))
+ sockPath := fmt.Sprintf("unix://%s/echo_%v.sock", t.TempDir(), tmrand.Str(6))
logger := log.NewNopLogger()
client, err := abciclient.NewClient(logger, sockPath, SOCKET, true)
if err != nil {
@@ -98,7 +98,7 @@ func TestEcho(t *testing.T) {
func BenchmarkEcho(b *testing.B) {
b.StopTimer() // Initialize
- sockPath := fmt.Sprintf("unix:///tmp/echo_%v.sock", tmrand.Str(6))
+ sockPath := fmt.Sprintf("unix://%s/echo_%v.sock", b.TempDir(), tmrand.Str(6))
logger := log.NewNopLogger()
client, err := abciclient.NewClient(logger, sockPath, SOCKET, true)
if err != nil {
@@ -146,7 +146,7 @@ func TestInfo(t *testing.T) {
ctx, cancel := context.WithCancel(context.Background())
defer cancel()
- sockPath := fmt.Sprintf("unix:///tmp/echo_%v.sock", tmrand.Str(6))
+ sockPath := fmt.Sprintf("unix://%s/echo_%v.sock", t.TempDir(), tmrand.Str(6))
logger := log.NewNopLogger()
client, err := abciclient.NewClient(logger, sockPath, SOCKET, true)
if err != nil {
From ad73e6da2f3d8f4cb00a0c4649ba06b941b40ec9 Mon Sep 17 00:00:00 2001
From: William Banfield <4561443+williambanfield@users.noreply.github.com>
Date: Thu, 19 May 2022 15:35:30 -0400
Subject: [PATCH 37/39] consensus: update state from store before use in
reactor (#8576)
Closes: #8575
This PR aims to fix the `LastCommitRound can only be negative for initial height 0` issue we see in the e2e tests by initializing the `state` object before starting the receive routines in the consensus reactor. This is somewhat inelegant, but it should fix the issue.
---
internal/consensus/reactor.go | 2 ++
internal/consensus/state.go | 18 +++++++++++-------
2 files changed, 13 insertions(+), 7 deletions(-)
diff --git a/internal/consensus/reactor.go b/internal/consensus/reactor.go
index 1a9d49057..501523339 100644
--- a/internal/consensus/reactor.go
+++ b/internal/consensus/reactor.go
@@ -216,6 +216,8 @@ func (r *Reactor) OnStart(ctx context.Context) error {
if err := r.state.Start(ctx); err != nil {
return err
}
+ } else if err := r.state.updateStateFromStore(); err != nil {
+ return err
}
go r.updateRoundStateRoutine(ctx)
diff --git a/internal/consensus/state.go b/internal/consensus/state.go
index b016e2687..320042d30 100644
--- a/internal/consensus/state.go
+++ b/internal/consensus/state.go
@@ -122,9 +122,8 @@ type State struct {
// store blocks and commits
blockStore sm.BlockStore
- stateStore sm.Store
- initialStatePopulated bool
- skipBootstrapping bool
+ stateStore sm.Store
+ skipBootstrapping bool
// create and execute blocks
blockExec *sm.BlockExecutor
@@ -248,9 +247,6 @@ func NewState(
}
func (cs *State) updateStateFromStore() error {
- if cs.initialStatePopulated {
- return nil
- }
state, err := cs.stateStore.Load()
if err != nil {
return fmt.Errorf("loading state: %w", err)
@@ -259,6 +255,15 @@ func (cs *State) updateStateFromStore() error {
return nil
}
+ eq, err := state.Equals(cs.state)
+ if err != nil {
+ return fmt.Errorf("comparing state: %w", err)
+ }
+ // if the new state is equivalent to the old state, we should not trigger a state update.
+ if eq {
+ return nil
+ }
+
// We have no votes, so reconstruct LastCommit from SeenCommit.
if state.LastBlockHeight > 0 {
cs.reconstructLastCommit(state)
@@ -266,7 +271,6 @@ func (cs *State) updateStateFromStore() error {
cs.updateToState(state)
- cs.initialStatePopulated = true
return nil
}
From 4786a5ffded3a59865772242266b84c736cf01ff Mon Sep 17 00:00:00 2001
From: "M. J. Fromberger"
Date: Thu, 19 May 2022 15:23:14 -0700
Subject: [PATCH 38/39] Remove the periodically-scheduled Markdown link check.
(#8580)
The state of links in our documentation is now sufficiently good that we are
running link checks during PRs. There is no longer any practical benefit to
running the scheduled "global" check. Most of the errors it reports are rate
limitations anyway (429).
---
.github/workflows/linkchecker.yml | 12 ------------
1 file changed, 12 deletions(-)
delete mode 100644 .github/workflows/linkchecker.yml
diff --git a/.github/workflows/linkchecker.yml b/.github/workflows/linkchecker.yml
deleted file mode 100644
index e2ba80861..000000000
--- a/.github/workflows/linkchecker.yml
+++ /dev/null
@@ -1,12 +0,0 @@
-name: Check Markdown links
-on:
- schedule:
- - cron: '* */24 * * *'
-jobs:
- markdown-link-check:
- runs-on: ubuntu-latest
- steps:
- - uses: actions/checkout@v3
- - uses: creachadair/github-action-markdown-link-check@master
- with:
- folder-path: "docs"
From 0cceadf4d4c5acf9d390f22616b80070fece52d4 Mon Sep 17 00:00:00 2001
From: William Banfield <4561443+williambanfield@users.noreply.github.com>
Date: Fri, 20 May 2022 17:46:52 -0400
Subject: [PATCH 39/39] abci++: add consensus parameter logic to control vote
extension require height (#8547)
This PR makes vote extensions optional within Tendermint. A new ConsensusParams field, called ABCIParams.VoteExtensionsEnableHeight, has been added to toggle whether or not extensions should be enabled or disabled depending on the current height of the consensus engine. Related to: #8453
---
internal/blocksync/pool.go | 6 +-
internal/blocksync/reactor.go | 121 +++++---
internal/blocksync/reactor_test.go | 181 +++++++----
internal/consensus/common_test.go | 15 +-
internal/consensus/msgs.go | 6 +-
internal/consensus/reactor.go | 21 +-
internal/consensus/reactor_test.go | 113 +++++++
internal/consensus/replay_test.go | 8 +-
internal/consensus/state.go | 148 ++++++---
internal/consensus/state_test.go | 285 +++++++++++++-----
internal/consensus/types/height_vote_set.go | 26 +-
.../consensus/types/height_vote_set_test.go | 2 +-
internal/evidence/pool_test.go | 7 +-
internal/evidence/verify_test.go | 24 +-
internal/state/execution.go | 22 +-
internal/state/execution_test.go | 112 ++++++-
internal/state/mocks/block_store.go | 7 +-
internal/state/services.go | 3 +-
internal/state/store.go | 19 ++
internal/state/validation_test.go | 6 +-
internal/statesync/reactor_test.go | 4 +-
internal/store/store.go | 90 ++++--
internal/store/store_test.go | 116 ++++++-
internal/test/factory/params.go | 1 +
node/node_test.go | 2 +-
test/e2e/runner/evidence.go | 6 +-
test/e2e/tests/app_test.go | 1 +
types/block.go | 109 ++++++-
types/block_test.go | 159 ++++++++--
types/evidence_test.go | 6 +-
types/params.go | 24 ++
types/validation_test.go | 10 +-
types/validator_set_test.go | 6 +-
types/vote.go | 71 +++--
types/vote_set.go | 38 ++-
types/vote_set_test.go | 90 +++++-
types/vote_test.go | 50 ++-
37 files changed, 1509 insertions(+), 406 deletions(-)
diff --git a/internal/blocksync/pool.go b/internal/blocksync/pool.go
index 30bb6962e..64ce54dc6 100644
--- a/internal/blocksync/pool.go
+++ b/internal/blocksync/pool.go
@@ -280,7 +280,7 @@ func (pool *BlockPool) AddBlock(peerID types.NodeID, block *types.Block, extComm
pool.mtx.Lock()
defer pool.mtx.Unlock()
- if block.Height != extCommit.Height {
+ if extCommit != nil && block.Height != extCommit.Height {
return fmt.Errorf("heights don't match, not adding block (block height: %d, commit height: %d)", block.Height, extCommit.Height)
}
@@ -597,7 +597,9 @@ func (bpr *bpRequester) setBlock(block *types.Block, extCommit *types.ExtendedCo
return false
}
bpr.block = block
- bpr.extCommit = extCommit
+ if extCommit != nil {
+ bpr.extCommit = extCommit
+ }
bpr.mtx.Unlock()
select {
diff --git a/internal/blocksync/reactor.go b/internal/blocksync/reactor.go
index 144595889..6c1c060e7 100644
--- a/internal/blocksync/reactor.go
+++ b/internal/blocksync/reactor.go
@@ -185,31 +185,39 @@ func (r *Reactor) OnStop() {
// Otherwise, we'll respond saying we do not have it.
func (r *Reactor) respondToPeer(ctx context.Context, msg *bcproto.BlockRequest, peerID types.NodeID, blockSyncCh *p2p.Channel) error {
block := r.store.LoadBlock(msg.Height)
- if block != nil {
- extCommit := r.store.LoadBlockExtendedCommit(msg.Height)
- if extCommit == nil {
- return fmt.Errorf("found block in store without extended commit: %v", block)
- }
- blockProto, err := block.ToProto()
- if err != nil {
- return fmt.Errorf("failed to convert block to protobuf: %w", err)
- }
-
+ if block == nil {
+ r.logger.Info("peer requesting a block we do not have", "peer", peerID, "height", msg.Height)
return blockSyncCh.Send(ctx, p2p.Envelope{
- To: peerID,
- Message: &bcproto.BlockResponse{
- Block: blockProto,
- ExtCommit: extCommit.ToProto(),
- },
+ To: peerID,
+ Message: &bcproto.NoBlockResponse{Height: msg.Height},
})
}
- r.logger.Info("peer requesting a block we do not have", "peer", peerID, "height", msg.Height)
+ state, err := r.stateStore.Load()
+ if err != nil {
+ return fmt.Errorf("loading state: %w", err)
+ }
+ var extCommit *types.ExtendedCommit
+ if state.ConsensusParams.ABCI.VoteExtensionsEnabled(msg.Height) {
+ extCommit = r.store.LoadBlockExtendedCommit(msg.Height)
+ if extCommit == nil {
+ return fmt.Errorf("found block in store with no extended commit: %v", block)
+ }
+ }
+
+ blockProto, err := block.ToProto()
+ if err != nil {
+ return fmt.Errorf("failed to convert block to protobuf: %w", err)
+ }
return blockSyncCh.Send(ctx, p2p.Envelope{
- To: peerID,
- Message: &bcproto.NoBlockResponse{Height: msg.Height},
+ To: peerID,
+ Message: &bcproto.BlockResponse{
+ Block: blockProto,
+ ExtCommit: extCommit.ToProto(),
+ },
})
+
}
// handleMessage handles an Envelope sent from a peer on a specific p2p Channel.
@@ -242,12 +250,16 @@ func (r *Reactor) handleMessage(ctx context.Context, envelope *p2p.Envelope, blo
"err", err)
return err
}
- extCommit, err := types.ExtendedCommitFromProto(msg.ExtCommit)
- if err != nil {
- r.logger.Error("failed to convert extended commit from proto",
- "peer", envelope.From,
- "err", err)
- return err
+ var extCommit *types.ExtendedCommit
+ if msg.ExtCommit != nil {
+ var err error
+ extCommit, err = types.ExtendedCommitFromProto(msg.ExtCommit)
+ if err != nil {
+ r.logger.Error("failed to convert extended commit from proto",
+ "peer", envelope.From,
+ "err", err)
+ return err
+ }
}
if err := r.pool.AddBlock(envelope.From, block, extCommit, block.Size()); err != nil {
@@ -440,6 +452,8 @@ func (r *Reactor) poolRoutine(ctx context.Context, stateSynced bool, blockSyncCh
lastRate = 0.0
didProcessCh = make(chan struct{}, 1)
+
+ initialCommitHasExtensions = (r.initialState.LastBlockHeight > 0 && r.store.LoadBlockExtendedCommit(r.initialState.LastBlockHeight) != nil)
)
defer trySyncTicker.Stop()
@@ -463,12 +477,27 @@ func (r *Reactor) poolRoutine(ctx context.Context, stateSynced bool, blockSyncCh
)
switch {
- // TODO(sergio) Might be needed for implementing the upgrading solution. Remove after that
- //case state.LastBlockHeight > 0 && r.store.LoadBlockExtCommit(state.LastBlockHeight) == nil:
- case state.LastBlockHeight > 0 && blocksSynced == 0:
- // Having state-synced, we need to blocksync at least one block
+
+ // The case statement below is a bit confusing, so here is a breakdown
+ // of its logic and purpose:
+ //
+ // If VoteExtensions are enabled we cannot switch to consensus without
+ // the vote extension data for the previous height, i.e. state.LastBlockHeight.
+ //
+ // If extensions were required during state.LastBlockHeight and we have
+ // sync'd at least one block, then we are guaranteed to have extensions.
+ // BlockSync requires that the blocks it fetches have extensions if
+ // extensions were enabled during the height.
+ //
+ // If extensions were required during state.LastBlockHeight and we have
+ // not sync'd any blocks, then we can only transition to Consensus
+ // if we already had extensions for the initial height.
+ // If any of these conditions is not met, we continue the loop, looking
+ // for extensions.
+ case state.ConsensusParams.ABCI.VoteExtensionsEnabled(state.LastBlockHeight) &&
+ (blocksSynced == 0 && !initialCommitHasExtensions):
r.logger.Info(
- "no seen commit yet",
+ "no extended commit yet",
"height", height,
"last_block_height", state.LastBlockHeight,
"initial_height", state.InitialHeight,
@@ -520,18 +549,19 @@ func (r *Reactor) poolRoutine(ctx context.Context, stateSynced bool, blockSyncCh
// see if there are any blocks to sync
first, second, extCommit := r.pool.PeekTwoBlocks()
- if first == nil || second == nil || extCommit == nil {
- if first != nil && extCommit == nil {
- // See https://github.com/tendermint/tendermint/pull/8433#discussion_r866790631
- panic(fmt.Errorf("peeked first block without extended commit at height %d - possible node store corruption", first.Height))
- }
- // we need all to sync the first block
+ if first != nil && extCommit == nil &&
+ state.ConsensusParams.ABCI.VoteExtensionsEnabled(first.Height) {
+ // See https://github.com/tendermint/tendermint/pull/8433#discussion_r866790631
+ panic(fmt.Errorf("peeked first block without extended commit at height %d - possible node store corruption", first.Height))
+ } else if first == nil || second == nil {
+ // we need to have fetched two consecutive blocks in order to
+ // perform blocksync verification
continue
- } else {
- // try again quickly next loop
- didProcessCh <- struct{}{}
}
+ // try again quickly next loop
+ didProcessCh <- struct{}{}
+
firstParts, err := first.MakePartSet(types.BlockPartSizeBytes)
if err != nil {
r.logger.Error("failed to make ",
@@ -557,7 +587,10 @@ func (r *Reactor) poolRoutine(ctx context.Context, stateSynced bool, blockSyncCh
// validate the block before we persist it
err = r.blockExec.ValidateBlock(ctx, state, first)
}
-
+ if err == nil && state.ConsensusParams.ABCI.VoteExtensionsEnabled(first.Height) {
+ // if vote extensions were required at this height, ensure they exist.
+ err = extCommit.EnsureExtensions()
+ }
// If either of the checks failed we log the error and request for a new block
// at that height
if err != nil {
@@ -593,7 +626,15 @@ func (r *Reactor) poolRoutine(ctx context.Context, stateSynced bool, blockSyncCh
r.pool.PopRequest()
// TODO: batch saves so we do not persist to disk every block
- r.store.SaveBlock(first, firstParts, extCommit)
+ if state.ConsensusParams.ABCI.VoteExtensionsEnabled(first.Height) {
+ r.store.SaveBlockWithExtendedCommit(first, firstParts, extCommit)
+ } else {
+ // We use LastCommit here instead of extCommit. extCommit is not
+ // guaranteed to be populated by the peer if extensions are not enabled.
+ // Currently, the peer should provide an extCommit even if the vote extension data are absent
+ // but this may change so using second.LastCommit is safer.
+ r.store.SaveBlock(first, firstParts, second.LastCommit)
+ }
// TODO: Same thing for app - but we would need a way to get the hash
// without persisting the state.
diff --git a/internal/blocksync/reactor_test.go b/internal/blocksync/reactor_test.go
index 1d4d7d4d6..0477eb45d 100644
--- a/internal/blocksync/reactor_test.go
+++ b/internal/blocksync/reactor_test.go
@@ -40,8 +40,6 @@ type reactorTestSuite struct {
blockSyncChannels map[types.NodeID]*p2p.Channel
peerChans map[types.NodeID]chan p2p.PeerUpdate
peerUpdates map[types.NodeID]*p2p.PeerUpdates
-
- blockSync bool
}
func setup(
@@ -69,7 +67,6 @@ func setup(
blockSyncChannels: make(map[types.NodeID]*p2p.Channel, numNodes),
peerChans: make(map[types.NodeID]chan p2p.PeerUpdate, numNodes),
peerUpdates: make(map[types.NodeID]*p2p.PeerUpdates, numNodes),
- blockSync: true,
}
chDesc := &p2p.ChannelDescriptor{ID: BlockSyncChannel, MessageType: new(bcproto.Message)}
@@ -97,21 +94,19 @@ func setup(
return rts
}
-func (rts *reactorTestSuite) addNode(
+func makeReactor(
ctx context.Context,
t *testing.T,
nodeID types.NodeID,
genDoc *types.GenesisDoc,
privVal types.PrivValidator,
- maxBlockHeight int64,
-) {
- t.Helper()
+ channelCreator p2p.ChannelCreator,
+ peerEvents p2p.PeerEventSubscriber) *Reactor {
logger := log.NewNopLogger()
- rts.nodes = append(rts.nodes, nodeID)
- rts.app[nodeID] = proxy.New(abciclient.NewLocalClient(logger, &abci.BaseApplication{}), logger, proxy.NopMetrics())
- require.NoError(t, rts.app[nodeID].Start(ctx))
+ app := proxy.New(abciclient.NewLocalClient(logger, &abci.BaseApplication{}), logger, proxy.NopMetrics())
+ require.NoError(t, app.Start(ctx))
blockDB := dbm.NewMemDB()
stateDB := dbm.NewMemDB()
@@ -139,7 +134,7 @@ func (rts *reactorTestSuite) addNode(
blockExec := sm.NewBlockExecutor(
stateStore,
log.NewNopLogger(),
- rts.app[nodeID],
+ app,
mp,
sm.EmptyEvidencePool{},
blockStore,
@@ -147,44 +142,35 @@ func (rts *reactorTestSuite) addNode(
sm.NopMetrics(),
)
- var lastExtCommit *types.ExtendedCommit
+ return NewReactor(
+ logger,
+ stateStore,
+ blockExec,
+ blockStore,
+ nil,
+ channelCreator,
+ peerEvents,
+ true,
+ consensus.NopMetrics(),
+ nil, // eventbus, can be nil
+ )
+}
- // The commit we are building for the current height.
- seenExtCommit := &types.ExtendedCommit{}
+func (rts *reactorTestSuite) addNode(
+ ctx context.Context,
+ t *testing.T,
+ nodeID types.NodeID,
+ genDoc *types.GenesisDoc,
+ privVal types.PrivValidator,
+ maxBlockHeight int64,
+) {
+ t.Helper()
- for blockHeight := int64(1); blockHeight <= maxBlockHeight; blockHeight++ {
- lastExtCommit = seenExtCommit.Clone()
+ logger := log.NewNopLogger()
- thisBlock := sf.MakeBlock(state, blockHeight, lastExtCommit.StripExtensions())
- thisParts, err := thisBlock.MakePartSet(types.BlockPartSizeBytes)
- require.NoError(t, err)
- blockID := types.BlockID{Hash: thisBlock.Hash(), PartSetHeader: thisParts.Header()}
-
- // Simulate a commit for the current height
- vote, err := factory.MakeVote(
- ctx,
- privVal,
- thisBlock.Header.ChainID,
- 0,
- thisBlock.Header.Height,
- 0,
- 2,
- blockID,
- time.Now(),
- )
- require.NoError(t, err)
- seenExtCommit = &types.ExtendedCommit{
- Height: vote.Height,
- Round: vote.Round,
- BlockID: blockID,
- ExtendedSignatures: []types.ExtendedCommitSig{vote.ExtendedCommitSig()},
- }
-
- state, err = blockExec.ApplyBlock(ctx, state, blockID, thisBlock)
- require.NoError(t, err)
-
- blockStore.SaveBlock(thisBlock, thisParts, seenExtCommit)
- }
+ rts.nodes = append(rts.nodes, nodeID)
+ rts.app[nodeID] = proxy.New(abciclient.NewLocalClient(logger, &abci.BaseApplication{}), logger, proxy.NopMetrics())
+ require.NoError(t, rts.app[nodeID].Start(ctx))
rts.peerChans[nodeID] = make(chan p2p.PeerUpdate)
rts.peerUpdates[nodeID] = p2p.NewPeerUpdates(rts.peerChans[nodeID], 1)
@@ -193,21 +179,64 @@ func (rts *reactorTestSuite) addNode(
chCreator := func(ctx context.Context, chdesc *p2p.ChannelDescriptor) (*p2p.Channel, error) {
return rts.blockSyncChannels[nodeID], nil
}
- rts.reactors[nodeID] = NewReactor(
- rts.logger.With("nodeID", nodeID),
- stateStore,
- blockExec,
- blockStore,
- nil,
- chCreator,
- func(ctx context.Context) *p2p.PeerUpdates { return rts.peerUpdates[nodeID] },
- rts.blockSync,
- consensus.NopMetrics(),
- nil, // eventbus, can be nil
- )
- require.NoError(t, rts.reactors[nodeID].Start(ctx))
- require.True(t, rts.reactors[nodeID].IsRunning())
+ peerEvents := func(ctx context.Context) *p2p.PeerUpdates { return rts.peerUpdates[nodeID] }
+ reactor := makeReactor(ctx, t, nodeID, genDoc, privVal, chCreator, peerEvents)
+
+ lastExtCommit := &types.ExtendedCommit{}
+
+ state, err := reactor.stateStore.Load()
+ require.NoError(t, err)
+ for blockHeight := int64(1); blockHeight <= maxBlockHeight; blockHeight++ {
+ block, blockID, partSet, seenExtCommit := makeNextBlock(ctx, t, state, privVal, blockHeight, lastExtCommit)
+
+ state, err = reactor.blockExec.ApplyBlock(ctx, state, blockID, block)
+ require.NoError(t, err)
+
+ reactor.store.SaveBlockWithExtendedCommit(block, partSet, seenExtCommit)
+ lastExtCommit = seenExtCommit
+ }
+
+ rts.reactors[nodeID] = reactor
+ require.NoError(t, reactor.Start(ctx))
+ require.True(t, reactor.IsRunning())
+}
+
+func makeNextBlock(ctx context.Context,
+ t *testing.T,
+ state sm.State,
+ signer types.PrivValidator,
+ height int64,
+ lc *types.ExtendedCommit) (*types.Block, types.BlockID, *types.PartSet, *types.ExtendedCommit) {
+
+ lastExtCommit := lc.Clone()
+
+ block := sf.MakeBlock(state, height, lastExtCommit.ToCommit())
+ partSet, err := block.MakePartSet(types.BlockPartSizeBytes)
+ require.NoError(t, err)
+ blockID := types.BlockID{Hash: block.Hash(), PartSetHeader: partSet.Header()}
+
+ // Simulate a commit for the current height
+ vote, err := factory.MakeVote(
+ ctx,
+ signer,
+ block.Header.ChainID,
+ 0,
+ block.Header.Height,
+ 0,
+ 2,
+ blockID,
+ time.Now(),
+ )
+ require.NoError(t, err)
+ seenExtCommit := &types.ExtendedCommit{
+ Height: vote.Height,
+ Round: vote.Round,
+ BlockID: blockID,
+ ExtendedSignatures: []types.ExtendedCommitSig{vote.ExtendedCommitSig()},
+ }
+ return block, blockID, partSet, seenExtCommit
+
}
func (rts *reactorTestSuite) start(ctx context.Context, t *testing.T) {
@@ -415,3 +444,35 @@ func TestReactor_BadBlockStopsPeer(t *testing.T) {
len(rts.reactors[newNode.NodeID].pool.peers),
)
}
+
+/*
+func TestReactorReceivesNoExtendedCommit(t *testing.T) {
+ blockDB := dbm.NewMemDB()
+ stateDB := dbm.NewMemDB()
+ stateStore := sm.NewStore(stateDB)
+ blockStore := store.NewBlockStore(blockDB)
+ blockExec := sm.NewBlockExecutor(
+ stateStore,
+ log.NewNopLogger(),
+ rts.app[nodeID],
+ mp,
+ sm.EmptyEvidencePool{},
+ blockStore,
+ eventbus,
+ sm.NopMetrics(),
+ )
+ NewReactor(
+ log.NewNopLogger(),
+ stateStore,
+ blockExec,
+ blockStore,
+ nil,
+ chCreator,
+ func(ctx context.Context) *p2p.PeerUpdates { return rts.peerUpdates[nodeID] },
+ rts.blockSync,
+ consensus.NopMetrics(),
+ nil, // eventbus, can be nil
+ )
+
+}
+*/
diff --git a/internal/consensus/common_test.go b/internal/consensus/common_test.go
index 1dc92b33c..27fc39d6b 100644
--- a/internal/consensus/common_test.go
+++ b/internal/consensus/common_test.go
@@ -527,10 +527,11 @@ func loadPrivValidator(t *testing.T, cfg *config.Config) *privval.FilePV {
}
type makeStateArgs struct {
- config *config.Config
- logger log.Logger
- validators int
- application abci.Application
+ config *config.Config
+ consensusParams *types.ConsensusParams
+ logger log.Logger
+ validators int
+ application abci.Application
}
func makeState(ctx context.Context, t *testing.T, args makeStateArgs) (*State, []*validatorStub) {
@@ -551,9 +552,13 @@ func makeState(ctx context.Context, t *testing.T, args makeStateArgs) (*State, [
if args.logger == nil {
args.logger = log.NewNopLogger()
}
+ c := factory.ConsensusParams()
+ if args.consensusParams != nil {
+ c = args.consensusParams
+ }
state, privVals := makeGenesisState(ctx, t, args.config, genesisStateArgs{
- Params: factory.ConsensusParams(),
+ Params: c,
Validators: validators,
})
diff --git a/internal/consensus/msgs.go b/internal/consensus/msgs.go
index c59c06a41..1024c24ae 100644
--- a/internal/consensus/msgs.go
+++ b/internal/consensus/msgs.go
@@ -222,11 +222,7 @@ func (*VoteMessage) TypeTag() string { return "tendermint/Vote" }
// ValidateBasic checks whether the vote within the message is well-formed.
func (m *VoteMessage) ValidateBasic() error {
- // Here we validate votes with vote extensions, since we require vote
- // extensions to be sent in precommit messages during consensus. Prevote
- // messages should never have vote extensions, and this is also validated
- // here.
- return m.Vote.ValidateWithExtension()
+ return m.Vote.ValidateBasic()
}
// String returns a string representation.
diff --git a/internal/consensus/reactor.go b/internal/consensus/reactor.go
index 501523339..18d5851a4 100644
--- a/internal/consensus/reactor.go
+++ b/internal/consensus/reactor.go
@@ -798,13 +798,20 @@ func (r *Reactor) gossipVotesRoutine(ctx context.Context, ps *PeerState, voteCh
if blockStoreBase > 0 && prs.Height != 0 && rs.Height >= prs.Height+2 && prs.Height >= blockStoreBase {
// Load the block's extended commit for prs.Height, which contains precommit
// signatures for prs.Height.
- if ec := r.state.blockStore.LoadBlockExtendedCommit(prs.Height); ec != nil {
- if ok, err := r.pickSendVote(ctx, ps, ec, voteCh); err != nil {
- return
- } else if ok {
- logger.Debug("picked Catchup commit to send", "height", prs.Height)
- continue
- }
+ var ec *types.ExtendedCommit
+ if r.state.state.ConsensusParams.ABCI.VoteExtensionsEnabled(prs.Height) {
+ ec = r.state.blockStore.LoadBlockExtendedCommit(prs.Height)
+ } else {
+ ec = r.state.blockStore.LoadBlockCommit(prs.Height).WrappedExtendedCommit()
+ }
+ if ec == nil {
+ continue
+ }
+ if ok, err := r.pickSendVote(ctx, ps, ec, voteCh); err != nil {
+ return
+ } else if ok {
+ logger.Debug("picked Catchup commit to send", "height", prs.Height)
+ continue
}
}
diff --git a/internal/consensus/reactor_test.go b/internal/consensus/reactor_test.go
index c6a8869db..96cf800bd 100644
--- a/internal/consensus/reactor_test.go
+++ b/internal/consensus/reactor_test.go
@@ -32,6 +32,7 @@ import (
"github.com/tendermint/tendermint/internal/test/factory"
"github.com/tendermint/tendermint/libs/log"
tmcons "github.com/tendermint/tendermint/proto/tendermint/consensus"
+ tmproto "github.com/tendermint/tendermint/proto/tendermint/types"
"github.com/tendermint/tendermint/types"
)
@@ -600,6 +601,118 @@ func TestReactorCreatesBlockWhenEmptyBlocksFalse(t *testing.T) {
wg.Wait()
}
+// TestSwitchToConsensusVoteExtensions tests that the SwitchToConsensus correctly
+// checks for vote extension data when required.
+func TestSwitchToConsensusVoteExtensions(t *testing.T) {
+ for _, testCase := range []struct {
+ name string
+ storedHeight int64
+ initialRequiredHeight int64
+ includeExtensions bool
+ shouldPanic bool
+ }{
+ {
+ name: "no vote extensions but not required",
+ initialRequiredHeight: 0,
+ storedHeight: 2,
+ includeExtensions: false,
+ shouldPanic: false,
+ },
+ {
+ name: "no vote extensions but required this height",
+ initialRequiredHeight: 2,
+ storedHeight: 2,
+ includeExtensions: false,
+ shouldPanic: true,
+ },
+ {
+ name: "no vote extensions and required in future",
+ initialRequiredHeight: 3,
+ storedHeight: 2,
+ includeExtensions: false,
+ shouldPanic: false,
+ },
+ {
+ name: "no vote extensions and required previous height",
+ initialRequiredHeight: 1,
+ storedHeight: 2,
+ includeExtensions: false,
+ shouldPanic: true,
+ },
+ {
+ name: "vote extensions and required previous height",
+ initialRequiredHeight: 1,
+ storedHeight: 2,
+ includeExtensions: true,
+ shouldPanic: false,
+ },
+ } {
+ t.Run(testCase.name, func(t *testing.T) {
+ ctx, cancel := context.WithTimeout(context.Background(), time.Second*15)
+ defer cancel()
+ cs, vs := makeState(ctx, t, makeStateArgs{validators: 1})
+ validator := vs[0]
+ validator.Height = testCase.storedHeight
+
+ cs.state.LastBlockHeight = testCase.storedHeight
+ cs.state.LastValidators = cs.state.Validators.Copy()
+ cs.state.ConsensusParams.ABCI.VoteExtensionsEnableHeight = testCase.initialRequiredHeight
+
+ propBlock, err := cs.createProposalBlock(ctx)
+ require.NoError(t, err)
+
+ // Consensus is preparing to do the next height after the stored height.
+ cs.Height = testCase.storedHeight + 1
+ propBlock.Height = testCase.storedHeight
+ blockParts, err := propBlock.MakePartSet(types.BlockPartSizeBytes)
+ require.NoError(t, err)
+
+ var voteSet *types.VoteSet
+ if testCase.includeExtensions {
+ voteSet = types.NewExtendedVoteSet(cs.state.ChainID, testCase.storedHeight, 0, tmproto.PrecommitType, cs.state.Validators)
+ } else {
+ voteSet = types.NewVoteSet(cs.state.ChainID, testCase.storedHeight, 0, tmproto.PrecommitType, cs.state.Validators)
+ }
+ signedVote := signVote(ctx, t, validator, tmproto.PrecommitType, cs.state.ChainID, types.BlockID{
+ Hash: propBlock.Hash(),
+ PartSetHeader: blockParts.Header(),
+ })
+
+ if !testCase.includeExtensions {
+ signedVote.Extension = nil
+ signedVote.ExtensionSignature = nil
+ }
+
+ added, err := voteSet.AddVote(signedVote)
+ require.NoError(t, err)
+ require.True(t, added)
+
+ if testCase.includeExtensions {
+ cs.blockStore.SaveBlockWithExtendedCommit(propBlock, blockParts, voteSet.MakeExtendedCommit())
+ } else {
+ cs.blockStore.SaveBlock(propBlock, blockParts, voteSet.MakeExtendedCommit().ToCommit())
+ }
+ reactor := NewReactor(
+ log.NewNopLogger(),
+ cs,
+ nil,
+ nil,
+ cs.eventBus,
+ true,
+ NopMetrics(),
+ )
+
+ if testCase.shouldPanic {
+ assert.Panics(t, func() {
+ reactor.SwitchToConsensus(ctx, cs.state, false)
+ })
+ } else {
+ reactor.SwitchToConsensus(ctx, cs.state, false)
+ }
+ })
+ }
+}
+
func TestReactorRecordsVotesAndBlockParts(t *testing.T) {
ctx, cancel := context.WithTimeout(context.Background(), time.Minute)
defer cancel()
diff --git a/internal/consensus/replay_test.go b/internal/consensus/replay_test.go
index c8f04655b..99d3c17a1 100644
--- a/internal/consensus/replay_test.go
+++ b/internal/consensus/replay_test.go
@@ -1204,14 +1204,16 @@ func (bs *mockBlockStore) LoadBlockMeta(height int64) *types.BlockMeta {
}
}
func (bs *mockBlockStore) LoadBlockPart(height int64, index int) *types.Part { return nil }
-func (bs *mockBlockStore) SaveBlock(block *types.Block, blockParts *types.PartSet, seenCommit *types.ExtendedCommit) {
+func (bs *mockBlockStore) SaveBlockWithExtendedCommit(block *types.Block, blockParts *types.PartSet, seenCommit *types.ExtendedCommit) {
+}
+func (bs *mockBlockStore) SaveBlock(block *types.Block, blockParts *types.PartSet, seenCommit *types.Commit) {
}
func (bs *mockBlockStore) LoadBlockCommit(height int64) *types.Commit {
- return bs.extCommits[height-1].StripExtensions()
+ return bs.extCommits[height-1].ToCommit()
}
func (bs *mockBlockStore) LoadSeenCommit() *types.Commit {
- return bs.extCommits[len(bs.extCommits)-1].StripExtensions()
+ return bs.extCommits[len(bs.extCommits)-1].ToCommit()
}
func (bs *mockBlockStore) LoadBlockExtendedCommit(height int64) *types.ExtendedCommit {
return bs.extCommits[height-1]
diff --git a/internal/consensus/state.go b/internal/consensus/state.go
index 320042d30..3af775bb6 100644
--- a/internal/consensus/state.go
+++ b/internal/consensus/state.go
@@ -696,23 +696,54 @@ func (cs *State) sendInternalMessage(ctx context.Context, mi msgInfo) {
}
}
-// Reconstruct LastCommit from SeenCommit, which we saved along with the block,
-// (which happens even before saving the state)
+// Reconstruct the LastCommit from either SeenCommit or the ExtendedCommit. SeenCommit
+// and ExtendedCommit are saved along with the block. If VoteExtensions are required
+// the method will panic on an absent ExtendedCommit or an ExtendedCommit without
+// extension data.
func (cs *State) reconstructLastCommit(state sm.State) {
- extCommit := cs.blockStore.LoadBlockExtendedCommit(state.LastBlockHeight)
- if extCommit == nil {
- panic(fmt.Sprintf(
- "failed to reconstruct last commit; commit for height %v not found",
- state.LastBlockHeight,
- ))
+ extensionsEnabled := cs.state.ConsensusParams.ABCI.VoteExtensionsEnabled(state.LastBlockHeight)
+ if !extensionsEnabled {
+ votes, err := cs.votesFromSeenCommit(state)
+ if err != nil {
+ panic(fmt.Sprintf("failed to reconstruct last commit; %s", err))
+ }
+ cs.LastCommit = votes
+ return
}
- lastPrecommits := extCommit.ToVoteSet(state.ChainID, state.LastValidators)
- if !lastPrecommits.HasTwoThirdsMajority() {
- panic("failed to reconstruct last commit; does not have +2/3 maj")
+ votes, err := cs.votesFromExtendedCommit(state)
+ if err != nil {
+ panic(fmt.Sprintf("failed to reconstruct last extended commit; %s", err))
+ }
+ cs.LastCommit = votes
+}
+
+func (cs *State) votesFromExtendedCommit(state sm.State) (*types.VoteSet, error) {
+ ec := cs.blockStore.LoadBlockExtendedCommit(state.LastBlockHeight)
+ if ec == nil {
+ return nil, fmt.Errorf("extended commit for height %v not found", state.LastBlockHeight)
+ }
+ vs := ec.ToExtendedVoteSet(state.ChainID, state.LastValidators)
+ if !vs.HasTwoThirdsMajority() {
+ return nil, errors.New("extended commit does not have +2/3 majority")
+ }
+ return vs, nil
+}
+
+func (cs *State) votesFromSeenCommit(state sm.State) (*types.VoteSet, error) {
+ commit := cs.blockStore.LoadSeenCommit()
+ if commit == nil || commit.Height != state.LastBlockHeight {
+ commit = cs.blockStore.LoadBlockCommit(state.LastBlockHeight)
+ }
+ if commit == nil {
+ return nil, fmt.Errorf("commit for height %v not found", state.LastBlockHeight)
}
- cs.LastCommit = lastPrecommits
+ vs := commit.ToVoteSet(state.ChainID, state.LastValidators)
+ if !vs.HasTwoThirdsMajority() {
+ return nil, errors.New("commit does not have +2/3 majority")
+ }
+ return vs, nil
}
// Updates State and increments height to match that of state.
@@ -814,7 +845,11 @@ func (cs *State) updateToState(state sm.State) {
cs.ValidRound = -1
cs.ValidBlock = nil
cs.ValidBlockParts = nil
- cs.Votes = cstypes.NewHeightVoteSet(state.ChainID, height, validators)
+ if state.ConsensusParams.ABCI.VoteExtensionsEnabled(height) {
+ cs.Votes = cstypes.NewExtendedHeightVoteSet(state.ChainID, height, validators)
+ } else {
+ cs.Votes = cstypes.NewHeightVoteSet(state.ChainID, height, validators)
+ }
cs.CommitRound = -1
cs.LastValidators = state.LastValidators
cs.TriggeredTimeoutPrecommit = false
@@ -1925,8 +1960,12 @@ func (cs *State) finalizeCommit(ctx context.Context, height int64) {
if cs.blockStore.Height() < block.Height {
// NOTE: the seenCommit is local justification to commit this block,
// but may differ from the LastCommit included in the next block
- precommits := cs.Votes.Precommits(cs.CommitRound)
- cs.blockStore.SaveBlock(block, blockParts, precommits.MakeExtendedCommit())
+ seenExtendedCommit := cs.Votes.Precommits(cs.CommitRound).MakeExtendedCommit()
+ if cs.state.ConsensusParams.ABCI.VoteExtensionsEnabled(block.Height) {
+ cs.blockStore.SaveBlockWithExtendedCommit(block, blockParts, seenExtendedCommit)
+ } else {
+ cs.blockStore.SaveBlock(block, blockParts, seenExtendedCommit.ToCommit())
+ }
} else {
// Happens during replay if we already saved the block but didn't commit
logger.Debug("calling finalizeCommit on already stored block", "height", block.Height)
@@ -2341,13 +2380,45 @@ func (cs *State) addVote(
return
}
- // Verify VoteExtension if precommit and not nil
- // https://github.com/tendermint/tendermint/issues/8487
- if vote.Type == tmproto.PrecommitType && !vote.BlockID.IsNil() {
- err := cs.blockExec.VerifyVoteExtension(ctx, vote)
- cs.metrics.MarkVoteExtensionReceived(err == nil)
- if err != nil {
- return false, err
+ // Check to see if the chain is configured to extend votes.
+ if cs.state.ConsensusParams.ABCI.VoteExtensionsEnabled(cs.Height) {
+ // The chain is configured to extend votes, check that the vote is
+ // not for a nil block and verify the extensions signature against the
+ // corresponding public key.
+
+ var myAddr []byte
+ if cs.privValidatorPubKey != nil {
+ myAddr = cs.privValidatorPubKey.Address()
+ }
+ // Verify VoteExtension if precommit and not nil
+ // https://github.com/tendermint/tendermint/issues/8487
+ if vote.Type == tmproto.PrecommitType && !vote.BlockID.IsNil() &&
+ !bytes.Equal(vote.ValidatorAddress, myAddr) { // Skip the VerifyVoteExtension call if the vote was issued by this validator.
+
+ // The core fields of the vote message were already validated in the
+ // consensus reactor when the vote was received.
+ // Here, we verify the signature of the vote extension included in the vote
+ // message.
+ _, val := cs.state.Validators.GetByIndex(vote.ValidatorIndex)
+ if err := vote.VerifyExtension(cs.state.ChainID, val.PubKey); err != nil {
+ return false, err
+ }
+
+ err := cs.blockExec.VerifyVoteExtension(ctx, vote)
+ cs.metrics.MarkVoteExtensionReceived(err == nil)
+ if err != nil {
+ return false, err
+ }
+ }
+ } else {
+ // Vote extensions are not enabled on the network.
+ // strip the extension data from the vote in case any is present.
+ //
+ // TODO punish a peer if it sent a vote with an extension when the feature
+ // is disabled on the network.
+ // https://github.com/tendermint/tendermint/issues/8565
+ if stripped := vote.StripExtension(); stripped {
+ cs.logger.Error("vote included extension data but vote extensions are not enabled", "peer", peerID)
}
}
@@ -2496,18 +2567,18 @@ func (cs *State) signVote(
// If the signedMessageType is for precommit,
// use our local precommit Timeout as the max wait time for getting a singed commit. The same goes for prevote.
- timeout := cs.voteTimeout(cs.Round)
-
+ timeout := time.Second
if msgType == tmproto.PrecommitType && !vote.BlockID.IsNil() {
+ timeout = cs.voteTimeout(cs.Round)
// if the signedMessage type is for a non-nil precommit, add
// VoteExtension
- ext, err := cs.blockExec.ExtendVote(ctx, vote)
- if err != nil {
- return nil, err
+ if cs.state.ConsensusParams.ABCI.VoteExtensionsEnabled(cs.Height) {
+ ext, err := cs.blockExec.ExtendVote(ctx, vote)
+ if err != nil {
+ return nil, err
+ }
+ vote.Extension = ext
}
- vote.Extension = ext
- } else {
- timeout = time.Second
}
v := vote.ToProto()
@@ -2547,14 +2618,17 @@ func (cs *State) signAddVote(
// TODO: pass pubKey to signVote
vote, err := cs.signVote(ctx, msgType, hash, header)
- if err == nil {
- cs.sendInternalMessage(ctx, msgInfo{&VoteMessage{vote}, "", tmtime.Now()})
- cs.logger.Debug("signed and pushed vote", "height", cs.Height, "round", cs.Round, "vote", vote)
- return vote
+ if err != nil {
+ cs.logger.Error("failed signing vote", "height", cs.Height, "round", cs.Round, "vote", vote, "err", err)
+ return nil
}
-
- cs.logger.Error("failed signing vote", "height", cs.Height, "round", cs.Round, "vote", vote, "err", err)
- return nil
+ if !cs.state.ConsensusParams.ABCI.VoteExtensionsEnabled(vote.Height) {
+ // The signer will sign the extension, make sure to remove the data on the way out
+ vote.StripExtension()
+ }
+ cs.sendInternalMessage(ctx, msgInfo{&VoteMessage{vote}, "", tmtime.Now()})
+ cs.logger.Debug("signed and pushed vote", "height", cs.Height, "round", cs.Round, "vote", vote)
+ return vote
}
// updatePrivValidatorPubKey get's the private validator public key and
diff --git a/internal/consensus/state_test.go b/internal/consensus/state_test.go
index 6fa69a1a3..797dcf59c 100644
--- a/internal/consensus/state_test.go
+++ b/internal/consensus/state_test.go
@@ -18,6 +18,7 @@ import (
"github.com/tendermint/tendermint/internal/eventbus"
tmpubsub "github.com/tendermint/tendermint/internal/pubsub"
tmquery "github.com/tendermint/tendermint/internal/pubsub/query"
+ "github.com/tendermint/tendermint/internal/test/factory"
tmbytes "github.com/tendermint/tendermint/libs/bytes"
"github.com/tendermint/tendermint/libs/log"
tmrand "github.com/tendermint/tendermint/libs/rand"
@@ -2026,77 +2027,101 @@ func TestFinalizeBlockCalled(t *testing.T) {
}
}
-// TestExtendVoteCalled tests that the vote extension methods are called at the
-// correct point in the consensus algorithm.
-func TestExtendVoteCalled(t *testing.T) {
- config := configSetup(t)
- ctx, cancel := context.WithCancel(context.Background())
- defer cancel()
+// TestExtendVoteCalledWhenEnabled tests that the vote extension methods are called at the
+// correct point in the consensus algorithm when vote extensions are enabled.
+func TestExtendVoteCalledWhenEnabled(t *testing.T) {
+ for _, testCase := range []struct {
+ name string
+ enabled bool
+ }{
+ {
+ name: "enabled",
+ enabled: true,
+ },
+ {
+ name: "disabled",
+ enabled: false,
+ },
+ } {
+ t.Run(testCase.name, func(t *testing.T) {
+ config := configSetup(t)
+ ctx, cancel := context.WithCancel(context.Background())
+ defer cancel()
- m := abcimocks.NewApplication(t)
- m.On("ProcessProposal", mock.Anything, mock.Anything).Return(&abci.ResponseProcessProposal{Status: abci.ResponseProcessProposal_ACCEPT}, nil)
- m.On("PrepareProposal", mock.Anything, mock.Anything).Return(&abci.ResponsePrepareProposal{}, nil)
- m.On("ExtendVote", mock.Anything, mock.Anything).Return(&abci.ResponseExtendVote{
- VoteExtension: []byte("extension"),
- }, nil)
- m.On("VerifyVoteExtension", mock.Anything, mock.Anything).Return(&abci.ResponseVerifyVoteExtension{
- Status: abci.ResponseVerifyVoteExtension_ACCEPT,
- }, nil)
- m.On("Commit", mock.Anything).Return(&abci.ResponseCommit{}, nil).Maybe()
- m.On("FinalizeBlock", mock.Anything, mock.Anything).Return(&abci.ResponseFinalizeBlock{}, nil).Maybe()
- cs1, vss := makeState(ctx, t, makeStateArgs{config: config, application: m})
- height, round := cs1.Height, cs1.Round
+ m := abcimocks.NewApplication(t)
+ m.On("ProcessProposal", mock.Anything, mock.Anything).Return(&abci.ResponseProcessProposal{Status: abci.ResponseProcessProposal_ACCEPT}, nil)
+ m.On("PrepareProposal", mock.Anything, mock.Anything).Return(&abci.ResponsePrepareProposal{}, nil)
+ if testCase.enabled {
+ m.On("ExtendVote", mock.Anything, mock.Anything).Return(&abci.ResponseExtendVote{
+ VoteExtension: []byte("extension"),
+ }, nil)
+ m.On("VerifyVoteExtension", mock.Anything, mock.Anything).Return(&abci.ResponseVerifyVoteExtension{
+ Status: abci.ResponseVerifyVoteExtension_ACCEPT,
+ }, nil)
+ }
+ m.On("Commit", mock.Anything).Return(&abci.ResponseCommit{}, nil).Maybe()
+ m.On("FinalizeBlock", mock.Anything, mock.Anything).Return(&abci.ResponseFinalizeBlock{}, nil).Maybe()
+ c := factory.ConsensusParams()
+ if !testCase.enabled {
+ c.ABCI.VoteExtensionsEnableHeight = 0
+ }
+ cs1, vss := makeState(ctx, t, makeStateArgs{config: config, application: m, consensusParams: c})
+ height, round := cs1.Height, cs1.Round
- proposalCh := subscribe(ctx, t, cs1.eventBus, types.EventQueryCompleteProposal)
- newRoundCh := subscribe(ctx, t, cs1.eventBus, types.EventQueryNewRound)
- pv1, err := cs1.privValidator.GetPubKey(ctx)
- require.NoError(t, err)
- addr := pv1.Address()
- voteCh := subscribeToVoter(ctx, t, cs1, addr)
+ proposalCh := subscribe(ctx, t, cs1.eventBus, types.EventQueryCompleteProposal)
+ newRoundCh := subscribe(ctx, t, cs1.eventBus, types.EventQueryNewRound)
+ pv1, err := cs1.privValidator.GetPubKey(ctx)
+ require.NoError(t, err)
+ addr := pv1.Address()
+ voteCh := subscribeToVoter(ctx, t, cs1, addr)
- startTestRound(ctx, cs1, cs1.Height, round)
- ensureNewRound(t, newRoundCh, height, round)
- ensureNewProposal(t, proposalCh, height, round)
+ startTestRound(ctx, cs1, cs1.Height, round)
+ ensureNewRound(t, newRoundCh, height, round)
+ ensureNewProposal(t, proposalCh, height, round)
- m.AssertNotCalled(t, "ExtendVote", mock.Anything, mock.Anything)
+ m.AssertNotCalled(t, "ExtendVote", mock.Anything, mock.Anything)
- rs := cs1.GetRoundState()
+ rs := cs1.GetRoundState()
- blockID := types.BlockID{
- Hash: rs.ProposalBlock.Hash(),
- PartSetHeader: rs.ProposalBlockParts.Header(),
- }
- signAddVotes(ctx, t, cs1, tmproto.PrevoteType, config.ChainID(), blockID, vss[1:]...)
- ensurePrevoteMatch(t, voteCh, height, round, blockID.Hash)
+ blockID := types.BlockID{
+ Hash: rs.ProposalBlock.Hash(),
+ PartSetHeader: rs.ProposalBlockParts.Header(),
+ }
+ signAddVotes(ctx, t, cs1, tmproto.PrevoteType, config.ChainID(), blockID, vss[1:]...)
+ ensurePrevoteMatch(t, voteCh, height, round, blockID.Hash)
- ensurePrecommit(t, voteCh, height, round)
+ ensurePrecommit(t, voteCh, height, round)
- m.AssertCalled(t, "ExtendVote", ctx, &abci.RequestExtendVote{
- Height: height,
- Hash: blockID.Hash,
- })
+ if testCase.enabled {
+ m.AssertCalled(t, "ExtendVote", ctx, &abci.RequestExtendVote{
+ Height: height,
+ Hash: blockID.Hash,
+ })
+ } else {
+ m.AssertNotCalled(t, "ExtendVote", mock.Anything, mock.Anything)
+ }
- m.AssertCalled(t, "VerifyVoteExtension", ctx, &abci.RequestVerifyVoteExtension{
- Hash: blockID.Hash,
- ValidatorAddress: addr,
- Height: height,
- VoteExtension: []byte("extension"),
- })
- signAddVotes(ctx, t, cs1, tmproto.PrecommitType, config.ChainID(), blockID, vss[1:]...)
- ensureNewRound(t, newRoundCh, height+1, 0)
- m.AssertExpectations(t)
+ signAddVotes(ctx, t, cs1, tmproto.PrecommitType, config.ChainID(), blockID, vss[1:]...)
+ ensureNewRound(t, newRoundCh, height+1, 0)
+ m.AssertExpectations(t)
- // Only 3 of the vote extensions are seen, as consensus proceeds as soon as the +2/3 threshold
- // is observed by the consensus engine.
- for _, pv := range vss[:3] {
- pv, err := pv.GetPubKey(ctx)
- require.NoError(t, err)
- addr := pv.Address()
- m.AssertCalled(t, "VerifyVoteExtension", ctx, &abci.RequestVerifyVoteExtension{
- Hash: blockID.Hash,
- ValidatorAddress: addr,
- Height: height,
- VoteExtension: []byte("extension"),
+ // Only 3 of the vote extensions are seen, as consensus proceeds as soon as the +2/3 threshold
+ // is observed by the consensus engine.
+ for _, pv := range vss[1:3] {
+ pv, err := pv.GetPubKey(ctx)
+ require.NoError(t, err)
+ addr := pv.Address()
+ if testCase.enabled {
+ m.AssertCalled(t, "VerifyVoteExtension", ctx, &abci.RequestVerifyVoteExtension{
+ Hash: blockID.Hash,
+ ValidatorAddress: addr,
+ Height: height,
+ VoteExtension: []byte("extension"),
+ })
+ } else {
+ m.AssertNotCalled(t, "VerifyVoteExtension", mock.Anything, mock.Anything)
+ }
+ }
})
}
@@ -2121,6 +2146,7 @@ func TestVerifyVoteExtensionNotCalledOnAbsentPrecommit(t *testing.T) {
m.On("FinalizeBlock", mock.Anything, mock.Anything).Return(&abci.ResponseFinalizeBlock{}, nil).Maybe()
cs1, vss := makeState(ctx, t, makeStateArgs{config: config, application: m})
height, round := cs1.Height, cs1.Round
+ cs1.state.ConsensusParams.ABCI.VoteExtensionsEnableHeight = cs1.Height
proposalCh := subscribe(ctx, t, cs1.eventBus, types.EventQueryCompleteProposal)
newRoundCh := subscribe(ctx, t, cs1.eventBus, types.EventQueryNewRound)
@@ -2138,7 +2164,7 @@ func TestVerifyVoteExtensionNotCalledOnAbsentPrecommit(t *testing.T) {
Hash: rs.ProposalBlock.Hash(),
PartSetHeader: rs.ProposalBlockParts.Header(),
}
- signAddVotes(ctx, t, cs1, tmproto.PrevoteType, config.ChainID(), blockID, vss[2:]...)
+ signAddVotes(ctx, t, cs1, tmproto.PrevoteType, config.ChainID(), blockID, vss...)
ensurePrevoteMatch(t, voteCh, height, round, blockID.Hash)
ensurePrecommit(t, voteCh, height, round)
@@ -2148,13 +2174,6 @@ func TestVerifyVoteExtensionNotCalledOnAbsentPrecommit(t *testing.T) {
Hash: blockID.Hash,
})
- m.AssertCalled(t, "VerifyVoteExtension", mock.Anything, &abci.RequestVerifyVoteExtension{
- Hash: blockID.Hash,
- ValidatorAddress: addr,
- Height: height,
- VoteExtension: []byte("extension"),
- })
-
m.On("Commit", mock.Anything).Return(&abci.ResponseCommit{}, nil).Maybe()
signAddVotes(ctx, t, cs1, tmproto.PrecommitType, config.ChainID(), blockID, vss[2:]...)
ensureNewRound(t, newRoundCh, height+1, 0)
@@ -2266,6 +2285,134 @@ func TestPrepareProposalReceivesVoteExtensions(t *testing.T) {
}
}
+// TestVoteExtensionEnableHeight tests that 'ExtensionRequireHeight' correctly
+// enforces that vote extensions be present in consensus for heights greater than
+// or equal to the configured value.
+func TestVoteExtensionEnableHeight(t *testing.T) {
+ for _, testCase := range []struct {
+ name string
+ enableHeight int64
+ hasExtension bool
+ expectExtendCalled bool
+ expectVerifyCalled bool
+ expectSuccessfulRound bool
+ }{
+ {
+ name: "extension present but not enabled",
+ hasExtension: true,
+ enableHeight: 0,
+ expectExtendCalled: false,
+ expectVerifyCalled: false,
+ expectSuccessfulRound: true,
+ },
+ {
+ name: "extension absent but not required",
+ hasExtension: false,
+ enableHeight: 0,
+ expectExtendCalled: false,
+ expectVerifyCalled: false,
+ expectSuccessfulRound: true,
+ },
+ {
+ name: "extension present and required",
+ hasExtension: true,
+ enableHeight: 1,
+ expectExtendCalled: true,
+ expectVerifyCalled: true,
+ expectSuccessfulRound: true,
+ },
+ {
+ name: "extension absent but required",
+ hasExtension: false,
+ enableHeight: 1,
+ expectExtendCalled: true,
+ expectVerifyCalled: false,
+ expectSuccessfulRound: false,
+ },
+ {
+ name: "extension absent but required in future height",
+ hasExtension: false,
+ enableHeight: 2,
+ expectExtendCalled: false,
+ expectVerifyCalled: false,
+ expectSuccessfulRound: true,
+ },
+ } {
+ t.Run(testCase.name, func(t *testing.T) {
+ config := configSetup(t)
+ ctx, cancel := context.WithCancel(context.Background())
+ defer cancel()
+
+ numValidators := 3
+ m := abcimocks.NewApplication(t)
+ m.On("ProcessProposal", mock.Anything, mock.Anything).Return(&abci.ResponseProcessProposal{
+ Status: abci.ResponseProcessProposal_ACCEPT,
+ }, nil)
+ m.On("PrepareProposal", mock.Anything, mock.Anything).Return(&abci.ResponsePrepareProposal{}, nil)
+ if testCase.expectExtendCalled {
+ m.On("ExtendVote", mock.Anything, mock.Anything).Return(&abci.ResponseExtendVote{}, nil)
+ }
+ if testCase.expectVerifyCalled {
+ m.On("VerifyVoteExtension", mock.Anything, mock.Anything).Return(&abci.ResponseVerifyVoteExtension{
+ Status: abci.ResponseVerifyVoteExtension_ACCEPT,
+ }, nil).Times(numValidators - 1)
+ }
+ m.On("FinalizeBlock", mock.Anything, mock.Anything).Return(&abci.ResponseFinalizeBlock{}, nil).Maybe()
+ m.On("Commit", mock.Anything).Return(&abci.ResponseCommit{}, nil).Maybe()
+ c := factory.ConsensusParams()
+ c.ABCI.VoteExtensionsEnableHeight = testCase.enableHeight
+ cs1, vss := makeState(ctx, t, makeStateArgs{config: config, application: m, validators: numValidators, consensusParams: c})
+ cs1.state.ConsensusParams.ABCI.VoteExtensionsEnableHeight = testCase.enableHeight
+ height, round := cs1.Height, cs1.Round
+
+ timeoutCh := subscribe(ctx, t, cs1.eventBus, types.EventQueryTimeoutPropose)
+ proposalCh := subscribe(ctx, t, cs1.eventBus, types.EventQueryCompleteProposal)
+ newRoundCh := subscribe(ctx, t, cs1.eventBus, types.EventQueryNewRound)
+ pv1, err := cs1.privValidator.GetPubKey(ctx)
+ require.NoError(t, err)
+ addr := pv1.Address()
+ voteCh := subscribeToVoter(ctx, t, cs1, addr)
+
+ startTestRound(ctx, cs1, cs1.Height, round)
+ ensureNewRound(t, newRoundCh, height, round)
+ ensureNewProposal(t, proposalCh, height, round)
+ rs := cs1.GetRoundState()
+
+ blockID := types.BlockID{
+ Hash: rs.ProposalBlock.Hash(),
+ PartSetHeader: rs.ProposalBlockParts.Header(),
+ }
+
+ // sign all of the votes
+ signAddVotes(ctx, t, cs1, tmproto.PrevoteType, config.ChainID(), blockID, vss[1:]...)
+ ensurePrevoteMatch(t, voteCh, height, round, rs.ProposalBlock.Hash())
+
+ var ext []byte
+ if testCase.hasExtension {
+ ext = []byte("extension")
+ }
+
+ for _, vs := range vss[1:] {
+ vote, err := vs.signVote(ctx, tmproto.PrecommitType, config.ChainID(), blockID, ext)
+ if !testCase.hasExtension {
+ vote.ExtensionSignature = nil
+ }
+ require.NoError(t, err)
+ addVotes(cs1, vote)
+ }
+ if testCase.expectSuccessfulRound {
+ ensurePrecommit(t, voteCh, height, round)
+ height++
+ ensureNewRound(t, newRoundCh, height, round)
+ } else {
+ ensureNoNewTimeout(t, timeoutCh, cs1.state.ConsensusParams.Timeout.VoteTimeout(round).Nanoseconds())
+ }
+
+ m.AssertExpectations(t)
+ })
+ }
+}
+
// 4 vals, 3 Nil Precommits at P0
// What we want:
// P0 waits for timeoutPrecommit before starting next round
diff --git a/internal/consensus/types/height_vote_set.go b/internal/consensus/types/height_vote_set.go
index 661c5120e..389c02356 100644
--- a/internal/consensus/types/height_vote_set.go
+++ b/internal/consensus/types/height_vote_set.go
@@ -38,9 +38,10 @@ We let each peer provide us with up to 2 unexpected "catchup" rounds.
One for their LastCommit round, and another for the official commit round.
*/
type HeightVoteSet struct {
- chainID string
- height int64
- valSet *types.ValidatorSet
+ chainID string
+ height int64
+ valSet *types.ValidatorSet
+ extensionsEnabled bool
mtx sync.Mutex
round int32 // max tracked round
@@ -50,7 +51,17 @@ type HeightVoteSet struct {
func NewHeightVoteSet(chainID string, height int64, valSet *types.ValidatorSet) *HeightVoteSet {
hvs := &HeightVoteSet{
- chainID: chainID,
+ chainID: chainID,
+ extensionsEnabled: false,
+ }
+ hvs.Reset(height, valSet)
+ return hvs
+}
+
+func NewExtendedHeightVoteSet(chainID string, height int64, valSet *types.ValidatorSet) *HeightVoteSet {
+ hvs := &HeightVoteSet{
+ chainID: chainID,
+ extensionsEnabled: true,
}
hvs.Reset(height, valSet)
return hvs
@@ -108,7 +119,12 @@ func (hvs *HeightVoteSet) addRound(round int32) {
}
// log.Debug("addRound(round)", "round", round)
prevotes := types.NewVoteSet(hvs.chainID, hvs.height, round, tmproto.PrevoteType, hvs.valSet)
- precommits := types.NewVoteSet(hvs.chainID, hvs.height, round, tmproto.PrecommitType, hvs.valSet)
+ var precommits *types.VoteSet
+ if hvs.extensionsEnabled {
+ precommits = types.NewExtendedVoteSet(hvs.chainID, hvs.height, round, tmproto.PrecommitType, hvs.valSet)
+ } else {
+ precommits = types.NewVoteSet(hvs.chainID, hvs.height, round, tmproto.PrecommitType, hvs.valSet)
+ }
hvs.roundVoteSets[round] = RoundVoteSet{
Prevotes: prevotes,
Precommits: precommits,
diff --git a/internal/consensus/types/height_vote_set_test.go b/internal/consensus/types/height_vote_set_test.go
index acffa794c..a2cfd84ec 100644
--- a/internal/consensus/types/height_vote_set_test.go
+++ b/internal/consensus/types/height_vote_set_test.go
@@ -27,7 +27,7 @@ func TestPeerCatchupRounds(t *testing.T) {
valSet, privVals := factory.ValidatorSet(ctx, t, 10, 1)
chainID := cfg.ChainID()
- hvs := NewHeightVoteSet(chainID, 1, valSet)
+ hvs := NewExtendedHeightVoteSet(chainID, 1, valSet)
vote999_0 := makeVoteHR(ctx, t, 1, 0, 999, privVals, chainID)
added, err := hvs.AddVote(vote999_0, "peer1")
diff --git a/internal/evidence/pool_test.go b/internal/evidence/pool_test.go
index 4047d3e7f..f80d13c7b 100644
--- a/internal/evidence/pool_test.go
+++ b/internal/evidence/pool_test.go
@@ -250,7 +250,7 @@ func TestEvidencePoolUpdate(t *testing.T) {
)
require.NoError(t, err)
lastExtCommit := makeExtCommit(height, val.PrivKey.PubKey().Address())
- block := types.MakeBlock(height+1, []types.Tx{}, lastExtCommit.StripExtensions(), []types.Evidence{ev})
+ block := types.MakeBlock(height+1, []types.Tx{}, lastExtCommit.ToCommit(), []types.Evidence{ev})
// update state (partially)
state.LastBlockHeight = height + 1
@@ -569,7 +569,7 @@ func initializeBlockStore(db dbm.DB, state sm.State, valAddr []byte) (*store.Blo
for i := int64(1); i <= state.LastBlockHeight; i++ {
lastCommit := makeExtCommit(i-1, valAddr)
- block := sf.MakeBlock(state, i, lastCommit.StripExtensions())
+ block := sf.MakeBlock(state, i, lastCommit.ToCommit())
block.Header.Time = defaultEvidenceTime.Add(time.Duration(i) * time.Minute)
block.Header.Version = version.Consensus{Block: version.BlockProtocol, App: 1}
@@ -580,7 +580,7 @@ func initializeBlockStore(db dbm.DB, state sm.State, valAddr []byte) (*store.Blo
}
seenCommit := makeExtCommit(i, valAddr)
- blockStore.SaveBlock(block, partSet, seenCommit)
+ blockStore.SaveBlockWithExtendedCommit(block, partSet, seenCommit)
}
return blockStore, nil
@@ -596,6 +596,7 @@ func makeExtCommit(height int64, valAddr []byte) *types.ExtendedCommit {
Timestamp: defaultEvidenceTime,
Signature: []byte("Signature"),
},
+ ExtensionSignature: []byte("Extended Signature"),
}},
}
}
diff --git a/internal/evidence/verify_test.go b/internal/evidence/verify_test.go
index 2ed84fa69..6341fde1d 100644
--- a/internal/evidence/verify_test.go
+++ b/internal/evidence/verify_test.go
@@ -233,10 +233,10 @@ func TestVerifyLightClientAttack_Equivocation(t *testing.T) {
// we are simulating a duplicate vote attack where all the validators in the conflictingVals set
// except the last validator vote twice
blockID := factory.MakeBlockIDWithHash(conflictingHeader.Hash())
- voteSet := types.NewVoteSet(evidenceChainID, 10, 1, tmproto.SignedMsgType(2), conflictingVals)
+ voteSet := types.NewExtendedVoteSet(evidenceChainID, 10, 1, tmproto.SignedMsgType(2), conflictingVals)
extCommit, err := factory.MakeExtendedCommit(ctx, blockID, 10, 1, voteSet, conflictingPrivVals[:4], defaultEvidenceTime)
require.NoError(t, err)
- commit := extCommit.StripExtensions()
+ commit := extCommit.ToCommit()
ev := &types.LightClientAttackEvidence{
ConflictingBlock: &types.LightBlock{
@@ -253,11 +253,11 @@ func TestVerifyLightClientAttack_Equivocation(t *testing.T) {
}
trustedBlockID := makeBlockID(trustedHeader.Hash(), 1000, []byte("partshash"))
- trustedVoteSet := types.NewVoteSet(evidenceChainID, 10, 1, tmproto.SignedMsgType(2), conflictingVals)
+ trustedVoteSet := types.NewExtendedVoteSet(evidenceChainID, 10, 1, tmproto.SignedMsgType(2), conflictingVals)
trustedExtCommit, err := factory.MakeExtendedCommit(ctx, trustedBlockID, 10, 1,
trustedVoteSet, conflictingPrivVals, defaultEvidenceTime)
require.NoError(t, err)
- trustedCommit := trustedExtCommit.StripExtensions()
+ trustedCommit := trustedExtCommit.ToCommit()
trustedSignedHeader := &types.SignedHeader{
Header: trustedHeader,
@@ -336,10 +336,10 @@ func TestVerifyLightClientAttack_Amnesia(t *testing.T) {
// we are simulating an amnesia attack where all the validators in the conflictingVals set
// except the last validator vote twice. However this time the commits are of different rounds.
blockID := makeBlockID(conflictingHeader.Hash(), 1000, []byte("partshash"))
- voteSet := types.NewVoteSet(evidenceChainID, height, 0, tmproto.SignedMsgType(2), conflictingVals)
+ voteSet := types.NewExtendedVoteSet(evidenceChainID, height, 0, tmproto.SignedMsgType(2), conflictingVals)
extCommit, err := factory.MakeExtendedCommit(ctx, blockID, height, 0, voteSet, conflictingPrivVals, defaultEvidenceTime)
require.NoError(t, err)
- commit := extCommit.StripExtensions()
+ commit := extCommit.ToCommit()
ev := &types.LightClientAttackEvidence{
ConflictingBlock: &types.LightBlock{
@@ -356,11 +356,11 @@ func TestVerifyLightClientAttack_Amnesia(t *testing.T) {
}
trustedBlockID := makeBlockID(trustedHeader.Hash(), 1000, []byte("partshash"))
- trustedVoteSet := types.NewVoteSet(evidenceChainID, height, 1, tmproto.SignedMsgType(2), conflictingVals)
+ trustedVoteSet := types.NewExtendedVoteSet(evidenceChainID, height, 1, tmproto.SignedMsgType(2), conflictingVals)
trustedExtCommit, err := factory.MakeExtendedCommit(ctx, trustedBlockID, height, 1,
trustedVoteSet, conflictingPrivVals, defaultEvidenceTime)
require.NoError(t, err)
- trustedCommit := trustedExtCommit.StripExtensions()
+ trustedCommit := trustedExtCommit.ToCommit()
trustedSignedHeader := &types.SignedHeader{
Header: trustedHeader,
@@ -553,10 +553,10 @@ func makeLunaticEvidence(
})
blockID := factory.MakeBlockIDWithHash(conflictingHeader.Hash())
- voteSet := types.NewVoteSet(evidenceChainID, height, 1, tmproto.SignedMsgType(2), conflictingVals)
+ voteSet := types.NewExtendedVoteSet(evidenceChainID, height, 1, tmproto.SignedMsgType(2), conflictingVals)
extCommit, err := factory.MakeExtendedCommit(ctx, blockID, height, 1, voteSet, conflictingPrivVals, defaultEvidenceTime)
require.NoError(t, err)
- commit := extCommit.StripExtensions()
+ commit := extCommit.ToCommit()
ev = &types.LightClientAttackEvidence{
ConflictingBlock: &types.LightBlock{
@@ -582,10 +582,10 @@ func makeLunaticEvidence(
}
trustedBlockID := factory.MakeBlockIDWithHash(trustedHeader.Hash())
trustedVals, privVals := factory.ValidatorSet(ctx, t, totalVals, defaultVotingPower)
- trustedVoteSet := types.NewVoteSet(evidenceChainID, height, 1, tmproto.SignedMsgType(2), trustedVals)
+ trustedVoteSet := types.NewExtendedVoteSet(evidenceChainID, height, 1, tmproto.SignedMsgType(2), trustedVals)
trustedExtCommit, err := factory.MakeExtendedCommit(ctx, trustedBlockID, height, 1, trustedVoteSet, privVals, defaultEvidenceTime)
require.NoError(t, err)
- trustedCommit := trustedExtCommit.StripExtensions()
+ trustedCommit := trustedExtCommit.ToCommit()
trusted = &types.LightBlock{
SignedHeader: &types.SignedHeader{
diff --git a/internal/state/execution.go b/internal/state/execution.go
index 2c88c793b..47c2cb7ae 100644
--- a/internal/state/execution.go
+++ b/internal/state/execution.go
@@ -3,6 +3,7 @@ package state
import (
"bytes"
"context"
+ "errors"
"fmt"
"time"
@@ -100,15 +101,14 @@ func (blockExec *BlockExecutor) CreateProposalBlock(
maxDataBytes := types.MaxDataBytes(maxBytes, evSize, state.Validators.Size())
txs := blockExec.mempool.ReapMaxBytesMaxGas(maxDataBytes, maxGas)
- commit := lastExtCommit.StripExtensions()
+ commit := lastExtCommit.ToCommit()
block := state.MakeBlock(height, txs, commit, evidence, proposerAddr)
-
rpp, err := blockExec.appClient.PrepareProposal(
ctx,
&abci.RequestPrepareProposal{
MaxTxBytes: maxDataBytes,
Txs: block.Txs.ToSliceOfBytes(),
- LocalLastCommit: buildExtendedCommitInfo(lastExtCommit, blockExec.store, state.InitialHeight),
+ LocalLastCommit: buildExtendedCommitInfo(lastExtCommit, blockExec.store, state.InitialHeight, state.ConsensusParams.ABCI),
ByzantineValidators: block.Evidence.ToABCI(),
Height: block.Height,
Time: block.Time,
@@ -321,7 +321,7 @@ func (blockExec *BlockExecutor) VerifyVoteExtension(ctx context.Context, vote *t
}
if !resp.IsOK() {
- return types.ErrVoteInvalidExtension
+ return errors.New("invalid vote extension")
}
return nil
@@ -428,7 +428,7 @@ func buildLastCommitInfo(block *types.Block, store Store, initialHeight int64) a
// data, it returns an empty record.
//
// Assumes that the commit signatures are sorted according to validator index.
-func buildExtendedCommitInfo(ec *types.ExtendedCommit, store Store, initialHeight int64) abci.ExtendedCommitInfo {
+func buildExtendedCommitInfo(ec *types.ExtendedCommit, store Store, initialHeight int64, ap types.ABCIParams) abci.ExtendedCommitInfo {
if ec.Height < initialHeight {
// There are no extended commits for heights below the initial height.
return abci.ExtendedCommitInfo{}
@@ -466,9 +466,15 @@ func buildExtendedCommitInfo(ec *types.ExtendedCommit, store Store, initialHeigh
}
var ext []byte
- if ecs.BlockIDFlag == types.BlockIDFlagCommit {
- // We only care about vote extensions if a validator has voted to
- // commit.
+ // Check if vote extensions were enabled during the commit's height: ec.Height.
+ // ec is the commit from the previous height, so if extensions were enabled
+ // during that height, we ensure they are present and deliver the data to
+ // the proposer. If they were not enabled during this previous height, we
+ // will not deliver extension data.
+ if ap.VoteExtensionsEnabled(ec.Height) && ecs.BlockIDFlag == types.BlockIDFlagCommit {
+ if err := ecs.EnsureExtension(); err != nil {
+ panic(fmt.Errorf("commit at height %d received with missing vote extensions data", ec.Height))
+ }
ext = ecs.Extension
}
diff --git a/internal/state/execution_test.go b/internal/state/execution_test.go
index ffe9cb6f8..5fb4dc297 100644
--- a/internal/state/execution_test.go
+++ b/internal/state/execution_test.go
@@ -140,7 +140,7 @@ func TestFinalizeBlockDecidedLastCommit(t *testing.T) {
}
// block for height 2
- block := sf.MakeBlock(state, 2, lastCommit.StripExtensions())
+ block := sf.MakeBlock(state, 2, lastCommit.ToCommit())
bps, err := block.MakePartSet(testPartSize)
require.NoError(t, err)
blockID := types.BlockID{Hash: block.Hash(), PartSetHeader: bps.Header()}
@@ -1004,6 +1004,116 @@ func TestPrepareProposalErrorOnPrepareProposalError(t *testing.T) {
mp.AssertExpectations(t)
}
+// TestCreateProposalBlockPanicOnAbsentVoteExtensions ensures that the CreateProposalBlock
+// call correctly panics when the vote extension data is missing from the extended commit
+// data that the method receives.
+func TestCreateProposalAbsentVoteExtensions(t *testing.T) {
+ for _, testCase := range []struct {
+ name string
+
+ // The height that is about to be proposed
+ height int64
+
+ // The first height during which vote extensions will be required for consensus to proceed.
+ extensionEnableHeight int64
+ expectPanic bool
+ }{
+ {
+ name: "missing extension data on first required height",
+ height: 2,
+ extensionEnableHeight: 1,
+ expectPanic: true,
+ },
+ {
+ name: "missing extension during before required height",
+ height: 2,
+ extensionEnableHeight: 2,
+ expectPanic: false,
+ },
+ {
+ name: "missing extension data and not required",
+ height: 2,
+ extensionEnableHeight: 0,
+ expectPanic: false,
+ },
+ {
+ name: "missing extension data and required in two heights",
+ height: 2,
+ extensionEnableHeight: 3,
+ expectPanic: false,
+ },
+ } {
+ t.Run(testCase.name, func(t *testing.T) {
+ ctx, cancel := context.WithCancel(context.Background())
+ defer cancel()
+
+ logger := log.NewNopLogger()
+
+ eventBus := eventbus.NewDefault(logger)
+ require.NoError(t, eventBus.Start(ctx))
+
+ app := abcimocks.NewApplication(t)
+ if !testCase.expectPanic {
+ app.On("PrepareProposal", mock.Anything, mock.Anything).Return(&abci.ResponsePrepareProposal{}, nil)
+ }
+ cc := abciclient.NewLocalClient(logger, app)
+ proxyApp := proxy.New(cc, logger, proxy.NopMetrics())
+ err := proxyApp.Start(ctx)
+ require.NoError(t, err)
+
+ state, stateDB, privVals := makeState(t, 1, int(testCase.height-1))
+ stateStore := sm.NewStore(stateDB)
+ state.ConsensusParams.ABCI.VoteExtensionsEnableHeight = testCase.extensionEnableHeight
+ mp := &mpmocks.Mempool{}
+ mp.On("Lock").Return()
+ mp.On("Unlock").Return()
+ mp.On("FlushAppConn", mock.Anything).Return(nil)
+ mp.On("Update",
+ mock.Anything,
+ mock.Anything,
+ mock.Anything,
+ mock.Anything,
+ mock.Anything,
+ mock.Anything).Return(nil)
+ mp.On("ReapMaxBytesMaxGas", mock.Anything, mock.Anything).Return(types.Txs{})
+
+ blockExec := sm.NewBlockExecutor(
+ stateStore,
+ logger,
+ proxyApp,
+ mp,
+ sm.EmptyEvidencePool{},
+ nil,
+ eventBus,
+ sm.NopMetrics(),
+ )
+ block := sf.MakeBlock(state, testCase.height, new(types.Commit))
+ bps, err := block.MakePartSet(testPartSize)
+ require.NoError(t, err)
+ blockID := types.BlockID{Hash: block.Hash(), PartSetHeader: bps.Header()}
+ pa, _ := state.Validators.GetByIndex(0)
+ lastCommit, _ := makeValidCommit(ctx, t, testCase.height-1, blockID, state.Validators, privVals)
+ stripSignatures(lastCommit)
+ if testCase.expectPanic {
+ require.Panics(t, func() {
+ blockExec.CreateProposalBlock(ctx, testCase.height, state, lastCommit, pa) //nolint:errcheck
+ })
+ } else {
+ _, err = blockExec.CreateProposalBlock(ctx, testCase.height, state, lastCommit, pa)
+ require.NoError(t, err)
+ }
+ })
+ }
+}
+
+func stripSignatures(ec *types.ExtendedCommit) {
+ for i, commitSig := range ec.ExtendedSignatures {
+ commitSig.Extension = nil
+ commitSig.ExtensionSignature = nil
+ ec.ExtendedSignatures[i] = commitSig
+ }
+}
+
func makeBlockID(hash []byte, partSetSize uint32, partSetHash []byte) types.BlockID {
var (
h = make([]byte, crypto.HashSize)
diff --git a/internal/state/mocks/block_store.go b/internal/state/mocks/block_store.go
index 4eafb1273..58fc640fc 100644
--- a/internal/state/mocks/block_store.go
+++ b/internal/state/mocks/block_store.go
@@ -209,7 +209,12 @@ func (_m *BlockStore) PruneBlocks(height int64) (uint64, error) {
}
// SaveBlock provides a mock function with given fields: block, blockParts, seenCommit
-func (_m *BlockStore) SaveBlock(block *types.Block, blockParts *types.PartSet, seenCommit *types.ExtendedCommit) {
+func (_m *BlockStore) SaveBlock(block *types.Block, blockParts *types.PartSet, seenCommit *types.Commit) {
+ _m.Called(block, blockParts, seenCommit)
+}
+
+// SaveBlockWithExtendedCommit provides a mock function with given fields: block, blockParts, seenCommit
+func (_m *BlockStore) SaveBlockWithExtendedCommit(block *types.Block, blockParts *types.PartSet, seenCommit *types.ExtendedCommit) {
_m.Called(block, blockParts, seenCommit)
}
diff --git a/internal/state/services.go b/internal/state/services.go
index 35a91aa11..f86c4e3cf 100644
--- a/internal/state/services.go
+++ b/internal/state/services.go
@@ -26,7 +26,8 @@ type BlockStore interface {
LoadBlockMeta(height int64) *types.BlockMeta
LoadBlock(height int64) *types.Block
- SaveBlock(block *types.Block, blockParts *types.PartSet, seenCommit *types.ExtendedCommit)
+ SaveBlock(block *types.Block, blockParts *types.PartSet, seenCommit *types.Commit)
+ SaveBlockWithExtendedCommit(block *types.Block, blockParts *types.PartSet, seenCommit *types.ExtendedCommit)
PruneBlocks(height int64) (uint64, error)
diff --git a/internal/state/store.go b/internal/state/store.go
index 2d2e4dc81..d592016f6 100644
--- a/internal/state/store.go
+++ b/internal/state/store.go
@@ -2,6 +2,7 @@ package state
import (
"bytes"
+ "encoding/binary"
"errors"
"fmt"
@@ -59,6 +60,7 @@ func abciResponsesKey(height int64) []byte {
// stateKey should never change after being set in init()
var stateKey []byte
+var tmpABCIKey []byte
func init() {
var err error
@@ -66,6 +68,12 @@ func init() {
if err != nil {
panic(err)
}
+ // temporary extra key before consensus param protos are regenerated
+ // TODO(wbanfield) remove in next PR
+ tmpABCIKey, err = orderedcode.Append(nil, int64(10000))
+ if err != nil {
+ panic(err)
+ }
}
//----------------------
@@ -137,6 +145,12 @@ func (store dbStore) loadState(key []byte) (state State, err error) {
if err != nil {
return state, err
}
+ buf, err = store.db.Get(tmpABCIKey)
+ if err != nil {
+ return state, err
+ }
+ h, _ := binary.Varint(buf)
+ sm.ConsensusParams.ABCI.VoteExtensionsEnableHeight = h
return *sm, nil
}
@@ -181,6 +195,11 @@ func (store dbStore) save(state State, key []byte) error {
if err := batch.Set(key, stateBz); err != nil {
return err
}
+ bz := make([]byte, 5)
+ binary.PutVarint(bz, state.ConsensusParams.ABCI.VoteExtensionsEnableHeight)
+ if err := batch.Set(tmpABCIKey, bz); err != nil {
+ return err
+ }
return batch.WriteSync()
}
diff --git a/internal/state/validation_test.go b/internal/state/validation_test.go
index b29cfd0f9..0f43db5eb 100644
--- a/internal/state/validation_test.go
+++ b/internal/state/validation_test.go
@@ -124,7 +124,7 @@ func TestValidateBlockHeader(t *testing.T) {
*/
state, _, lastExtCommit = makeAndCommitGoodBlock(ctx, t,
state, height, lastCommit, state.Validators.GetProposer().Address, blockExec, privVals, nil)
- lastCommit = lastExtCommit.StripExtensions()
+ lastCommit = lastExtCommit.ToCommit()
}
nextHeight := validationTestsStopHeight
@@ -234,7 +234,7 @@ func TestValidateBlockCommit(t *testing.T) {
privVals,
nil,
)
- lastCommit = lastExtCommit.StripExtensions()
+ lastCommit = lastExtCommit.ToCommit()
/*
wrongSigsCommit is fine except for the extra bad precommit
@@ -384,7 +384,7 @@ func TestValidateBlockEvidence(t *testing.T) {
privVals,
evidence,
)
- lastCommit = lastExtCommit.StripExtensions()
+ lastCommit = lastExtCommit.ToCommit()
}
}
diff --git a/internal/statesync/reactor_test.go b/internal/statesync/reactor_test.go
index 904fb2b74..f57e228a7 100644
--- a/internal/statesync/reactor_test.go
+++ b/internal/statesync/reactor_test.go
@@ -855,13 +855,13 @@ func mockLB(ctx context.Context, t *testing.T, height int64, time time.Time, las
header.NextValidatorsHash = nextVals.Hash()
header.ConsensusHash = types.DefaultConsensusParams().HashConsensusParams()
lastBlockID = factory.MakeBlockIDWithHash(header.Hash())
- voteSet := types.NewVoteSet(factory.DefaultTestChainID, height, 0, tmproto.PrecommitType, currentVals)
+ voteSet := types.NewExtendedVoteSet(factory.DefaultTestChainID, height, 0, tmproto.PrecommitType, currentVals)
extCommit, err := factory.MakeExtendedCommit(ctx, lastBlockID, height, 0, voteSet, currentPrivVals, time)
require.NoError(t, err)
return nextVals, nextPrivVals, &types.LightBlock{
SignedHeader: &types.SignedHeader{
Header: header,
- Commit: extCommit.StripExtensions(),
+ Commit: extCommit.ToCommit(),
},
ValidatorSet: currentVals,
}
diff --git a/internal/store/store.go b/internal/store/store.go
index 5617674a2..1ba7e398d 100644
--- a/internal/store/store.go
+++ b/internal/store/store.go
@@ -2,6 +2,7 @@ package store
import (
"bytes"
+ "errors"
"fmt"
"strconv"
@@ -278,6 +279,9 @@ func (bs *BlockStore) LoadBlockCommit(height int64) *types.Commit {
return commit
}
+// LoadExtendedCommit returns the ExtendedCommit for the given height.
+// The extended commit is not guaranteed to contain the same +2/3 precommits data
+// as the commit in the block.
func (bs *BlockStore) LoadBlockExtendedCommit(height int64) *types.ExtendedCommit {
pbec := new(tmproto.ExtendedCommit)
bz, err := bs.db.Get(extCommitKey(height))
@@ -466,25 +470,73 @@ func (bs *BlockStore) batchDelete(
// If all the nodes restart after committing a block,
// we need this to reload the precommits to catch-up nodes to the
// most recent height. Otherwise they'd stall at H-1.
-func (bs *BlockStore) SaveBlock(block *types.Block, blockParts *types.PartSet, seenCommit *types.ExtendedCommit) {
+func (bs *BlockStore) SaveBlock(block *types.Block, blockParts *types.PartSet, seenCommit *types.Commit) {
if block == nil {
panic("BlockStore can only save a non-nil block")
}
-
batch := bs.db.NewBatch()
+ if err := bs.saveBlockToBatch(batch, block, blockParts, seenCommit); err != nil {
+ panic(err)
+ }
+
+ if err := batch.WriteSync(); err != nil {
+ panic(err)
+ }
+
+ if err := batch.Close(); err != nil {
+ panic(err)
+ }
+}
+
+// SaveBlockWithExtendedCommit persists the given block, blockParts, and
+// seenExtendedCommit to the underlying db. seenExtendedCommit is stored under
+// two keys in the database: as the seenCommit and as the ExtendedCommit data for the
+// height. This allows the vote extension data to be persisted for all blocks
+// that are saved.
+func (bs *BlockStore) SaveBlockWithExtendedCommit(block *types.Block, blockParts *types.PartSet, seenExtendedCommit *types.ExtendedCommit) {
+ if block == nil {
+ panic("BlockStore can only save a non-nil block")
+ }
+ if err := seenExtendedCommit.EnsureExtensions(); err != nil {
+ panic(fmt.Errorf("saving block with extensions: %w", err))
+ }
+ batch := bs.db.NewBatch()
+ if err := bs.saveBlockToBatch(batch, block, blockParts, seenExtendedCommit.ToCommit()); err != nil {
+ panic(err)
+ }
+ height := block.Height
+
+ pbec := seenExtendedCommit.ToProto()
+ extCommitBytes := mustEncode(pbec)
+ if err := batch.Set(extCommitKey(height), extCommitBytes); err != nil {
+ panic(err)
+ }
+
+ if err := batch.WriteSync(); err != nil {
+ panic(err)
+ }
+
+ if err := batch.Close(); err != nil {
+ panic(err)
+ }
+}
+
+func (bs *BlockStore) saveBlockToBatch(batch dbm.Batch, block *types.Block, blockParts *types.PartSet, seenCommit *types.Commit) error {
+ if block == nil {
+ panic("BlockStore can only save a non-nil block")
+ }
height := block.Height
hash := block.Hash()
if g, w := height, bs.Height()+1; bs.Base() > 0 && g != w {
- panic(fmt.Sprintf("BlockStore can only save contiguous blocks. Wanted %v, got %v", w, g))
+ return fmt.Errorf("BlockStore can only save contiguous blocks. Wanted %v, got %v", w, g)
}
if !blockParts.IsComplete() {
- panic("BlockStore can only save complete block part sets")
+ return errors.New("BlockStore can only save complete block part sets")
}
if height != seenCommit.Height {
- panic(fmt.Sprintf("BlockStore cannot save seen commit of a different height (block: %d, commit: %d)",
- height, seenCommit.Height))
+ return fmt.Errorf("BlockStore cannot save seen commit of a different height (block: %d, commit: %d)", height, seenCommit.Height)
}
// Save block parts. This must be done before the block meta, since callers
@@ -499,44 +551,32 @@ func (bs *BlockStore) SaveBlock(block *types.Block, blockParts *types.PartSet, s
blockMeta := types.NewBlockMeta(block, blockParts)
pbm := blockMeta.ToProto()
if pbm == nil {
- panic("nil blockmeta")
+ return errors.New("nil blockmeta")
}
metaBytes := mustEncode(pbm)
if err := batch.Set(blockMetaKey(height), metaBytes); err != nil {
- panic(err)
+ return err
}
if err := batch.Set(blockHashKey(hash), []byte(fmt.Sprintf("%d", height))); err != nil {
- panic(err)
+ return err
}
pbc := block.LastCommit.ToProto()
blockCommitBytes := mustEncode(pbc)
if err := batch.Set(blockCommitKey(height-1), blockCommitBytes); err != nil {
- panic(err)
+ return err
}
// Save seen commit (seen +2/3 precommits for block)
- pbsc := seenCommit.StripExtensions().ToProto()
+ pbsc := seenCommit.ToProto()
seenCommitBytes := mustEncode(pbsc)
if err := batch.Set(seenCommitKey(), seenCommitBytes); err != nil {
- panic(err)
+ return err
}
- pbec := seenCommit.ToProto()
- extCommitBytes := mustEncode(pbec)
- if err := batch.Set(extCommitKey(height), extCommitBytes); err != nil {
- panic(err)
- }
-
- if err := batch.WriteSync(); err != nil {
- panic(err)
- }
-
- if err := batch.Close(); err != nil {
- panic(err)
- }
+ return nil
}
func (bs *BlockStore) saveBlockPart(height int64, index int, part *types.Part, batch dbm.Batch) {
diff --git a/internal/store/store_test.go b/internal/store/store_test.go
index 9df3eed9f..771129cc0 100644
--- a/internal/store/store_test.go
+++ b/internal/store/store_test.go
@@ -36,6 +36,7 @@ func makeTestExtCommit(height int64, timestamp time.Time) *types.ExtendedCommit
Timestamp: timestamp,
Signature: []byte("Signature"),
},
+ ExtensionSignature: []byte("ExtensionSignature"),
}}
return &types.ExtendedCommit{
Height: height,
@@ -89,7 +90,7 @@ func TestBlockStoreSaveLoadBlock(t *testing.T) {
part2 := validPartSet.GetPart(1)
seenCommit := makeTestExtCommit(block.Header.Height, tmtime.Now())
- bs.SaveBlock(block, validPartSet, seenCommit)
+ bs.SaveBlockWithExtendedCommit(block, validPartSet, seenCommit)
require.EqualValues(t, 1, bs.Base(), "expecting the new height to be changed")
require.EqualValues(t, block.Header.Height, bs.Height(), "expecting the new height to be changed")
@@ -107,7 +108,7 @@ func TestBlockStoreSaveLoadBlock(t *testing.T) {
}
// End of setup, test data
- commitAtH10 := makeTestExtCommit(10, tmtime.Now()).StripExtensions()
+ commitAtH10 := makeTestExtCommit(10, tmtime.Now()).ToCommit()
tuples := []struct {
block *types.Block
parts *types.PartSet
@@ -140,16 +141,17 @@ func TestBlockStoreSaveLoadBlock(t *testing.T) {
ChainID: "block_test",
Time: tmtime.Now(),
ProposerAddress: tmrand.Bytes(crypto.AddressSize)},
- makeTestExtCommit(5, tmtime.Now()).StripExtensions(),
+ makeTestExtCommit(5, tmtime.Now()).ToCommit(),
),
parts: validPartSet,
seenCommit: makeTestExtCommit(5, tmtime.Now()),
},
{
- block: newBlock(header1, commitAtH10),
- parts: incompletePartSet,
- wantPanic: "only save complete block", // incomplete parts
+ block: newBlock(header1, commitAtH10),
+ parts: incompletePartSet,
+ wantPanic: "only save complete block", // incomplete parts
+ seenCommit: makeTestExtCommit(10, tmtime.Now()),
},
{
@@ -178,7 +180,7 @@ func TestBlockStoreSaveLoadBlock(t *testing.T) {
},
{
- block: newBlock(header1, commitAtH10),
+ block: block,
parts: validPartSet,
seenCommit: seenCommit,
@@ -187,7 +189,7 @@ func TestBlockStoreSaveLoadBlock(t *testing.T) {
},
{
- block: newBlock(header1, commitAtH10),
+ block: block,
parts: validPartSet,
seenCommit: seenCommit,
@@ -209,7 +211,7 @@ func TestBlockStoreSaveLoadBlock(t *testing.T) {
bs, db := newInMemoryBlockStore()
// SaveBlock
res, err, panicErr := doFn(func() (interface{}, error) {
- bs.SaveBlock(tuple.block, tuple.parts, tuple.seenCommit)
+ bs.SaveBlockWithExtendedCommit(tuple.block, tuple.parts, tuple.seenCommit)
if tuple.block == nil {
return nil, nil
}
@@ -279,6 +281,90 @@ func TestBlockStoreSaveLoadBlock(t *testing.T) {
}
}
+// TestSaveBlockWithExtendedCommitPanicOnAbsentExtension tests that saving a
+// block with an extended commit panics when the extension data is absent.
+func TestSaveBlockWithExtendedCommitPanicOnAbsentExtension(t *testing.T) {
+ for _, testCase := range []struct {
+ name string
+ malleateCommit func(*types.ExtendedCommit)
+ shouldPanic bool
+ }{
+ {
+ name: "basic save",
+ malleateCommit: func(_ *types.ExtendedCommit) {},
+ shouldPanic: false,
+ },
+ {
+ name: "save commit with no extensions",
+ malleateCommit: func(c *types.ExtendedCommit) {
+ c.StripExtensions()
+ },
+ shouldPanic: true,
+ },
+ } {
+ t.Run(testCase.name, func(t *testing.T) {
+ state, bs, cleanup, err := makeStateAndBlockStore(t.TempDir())
+ require.NoError(t, err)
+ defer cleanup()
+ block := factory.MakeBlock(state, bs.Height()+1, new(types.Commit))
+ seenCommit := makeTestExtCommit(block.Header.Height, tmtime.Now())
+ ps, err := block.MakePartSet(2)
+ require.NoError(t, err)
+ testCase.malleateCommit(seenCommit)
+ if testCase.shouldPanic {
+ require.Panics(t, func() {
+ bs.SaveBlockWithExtendedCommit(block, ps, seenCommit)
+ })
+ } else {
+ bs.SaveBlockWithExtendedCommit(block, ps, seenCommit)
+ }
+ })
+ }
+}
+
+// TestLoadBlockExtendedCommit tests loading the extended commit for a previously
+// saved block. The load method should return nil when only a commit was saved and
+// return the extended commit otherwise.
+func TestLoadBlockExtendedCommit(t *testing.T) {
+ for _, testCase := range []struct {
+ name string
+ saveExtended bool
+ expectResult bool
+ }{
+ {
+ name: "save commit",
+ saveExtended: false,
+ expectResult: false,
+ },
+ {
+ name: "save extended commit",
+ saveExtended: true,
+ expectResult: true,
+ },
+ } {
+ t.Run(testCase.name, func(t *testing.T) {
+ state, bs, cleanup, err := makeStateAndBlockStore(t.TempDir())
+ require.NoError(t, err)
+ defer cleanup()
+ block := factory.MakeBlock(state, bs.Height()+1, new(types.Commit))
+ seenCommit := makeTestExtCommit(block.Header.Height, tmtime.Now())
+ ps, err := block.MakePartSet(2)
+ require.NoError(t, err)
+ if testCase.saveExtended {
+ bs.SaveBlockWithExtendedCommit(block, ps, seenCommit)
+ } else {
+ bs.SaveBlock(block, ps, seenCommit.ToCommit())
+ }
+ res := bs.LoadBlockExtendedCommit(block.Height)
+ if testCase.expectResult {
+ require.Equal(t, seenCommit, res)
+ } else {
+ require.Nil(t, res)
+ }
+ })
+ }
+}
+
func TestLoadBaseMeta(t *testing.T) {
cfg, err := config.ResetTestRoot(t.TempDir(), "blockchain_reactor_test")
require.NoError(t, err)
@@ -293,7 +379,7 @@ func TestLoadBaseMeta(t *testing.T) {
partSet, err := block.MakePartSet(2)
require.NoError(t, err)
seenCommit := makeTestExtCommit(h, tmtime.Now())
- bs.SaveBlock(block, partSet, seenCommit)
+ bs.SaveBlockWithExtendedCommit(block, partSet, seenCommit)
}
pruned, err := bs.PruneBlocks(4)
@@ -371,7 +457,7 @@ func TestPruneBlocks(t *testing.T) {
partSet, err := block.MakePartSet(2)
require.NoError(t, err)
seenCommit := makeTestExtCommit(h, tmtime.Now())
- bs.SaveBlock(block, partSet, seenCommit)
+ bs.SaveBlockWithExtendedCommit(block, partSet, seenCommit)
}
assert.EqualValues(t, 1, bs.Base())
@@ -479,7 +565,7 @@ func TestBlockFetchAtHeight(t *testing.T) {
partSet, err := block.MakePartSet(2)
require.NoError(t, err)
seenCommit := makeTestExtCommit(block.Header.Height, tmtime.Now())
- bs.SaveBlock(block, partSet, seenCommit)
+ bs.SaveBlockWithExtendedCommit(block, partSet, seenCommit)
require.Equal(t, bs.Height(), block.Header.Height, "expecting the new height to be changed")
blockAtHeight := bs.LoadBlock(bs.Height())
@@ -518,16 +604,16 @@ func TestSeenAndCanonicalCommit(t *testing.T) {
// produce a few blocks and check that the correct seen and cannoncial commits
// are persisted.
for h := int64(3); h <= 5; h++ {
- blockCommit := makeTestExtCommit(h-1, tmtime.Now()).StripExtensions()
+ blockCommit := makeTestExtCommit(h-1, tmtime.Now()).ToCommit()
block := factory.MakeBlock(state, h, blockCommit)
partSet, err := block.MakePartSet(2)
require.NoError(t, err)
seenCommit := makeTestExtCommit(h, tmtime.Now())
- store.SaveBlock(block, partSet, seenCommit)
+ store.SaveBlockWithExtendedCommit(block, partSet, seenCommit)
c3 := store.LoadSeenCommit()
require.NotNil(t, c3)
require.Equal(t, h, c3.Height)
- require.Equal(t, seenCommit.StripExtensions().Hash(), c3.Hash())
+ require.Equal(t, seenCommit.ToCommit().Hash(), c3.Hash())
c5 := store.LoadBlockCommit(h)
require.Nil(t, c5)
c6 := store.LoadBlockCommit(h - 1)
diff --git a/internal/test/factory/params.go b/internal/test/factory/params.go
index dda8e2b3c..c6fa3f9fc 100644
--- a/internal/test/factory/params.go
+++ b/internal/test/factory/params.go
@@ -18,5 +18,6 @@ func ConsensusParams() *types.ConsensusParams {
VoteDelta: 1 * time.Millisecond,
BypassCommitTimeout: true,
}
+ c.ABCI.VoteExtensionsEnableHeight = 1
return c
}
diff --git a/node/node_test.go b/node/node_test.go
index b1d7a9481..245e39b3c 100644
--- a/node/node_test.go
+++ b/node/node_test.go
@@ -526,7 +526,7 @@ func TestMaxProposalBlockSize(t *testing.T) {
}
state.ChainID = maxChainID
- voteSet := types.NewVoteSet(state.ChainID, math.MaxInt64-1, math.MaxInt32, tmproto.PrecommitType, state.Validators)
+ voteSet := types.NewExtendedVoteSet(state.ChainID, math.MaxInt64-1, math.MaxInt32, tmproto.PrecommitType, state.Validators)
// add maximum amount of signatures to a single commit
for i := 0; i < types.MaxVotesCount; i++ {
diff --git a/test/e2e/runner/evidence.go b/test/e2e/runner/evidence.go
index a71ea14fb..fab1d7b20 100644
--- a/test/e2e/runner/evidence.go
+++ b/test/e2e/runner/evidence.go
@@ -165,9 +165,9 @@ func generateLightClientAttackEvidence(
// create a commit for the forged header
blockID := makeBlockID(header.Hash(), 1000, []byte("partshash"))
- voteSet := types.NewVoteSet(chainID, forgedHeight, 0, tmproto.SignedMsgType(2), conflictingVals)
+ voteSet := types.NewExtendedVoteSet(chainID, forgedHeight, 0, tmproto.SignedMsgType(2), conflictingVals)
- commit, err := factory.MakeExtendedCommit(ctx, blockID, forgedHeight, 0, voteSet, pv, forgedTime)
+ ec, err := factory.MakeExtendedCommit(ctx, blockID, forgedHeight, 0, voteSet, pv, forgedTime)
if err != nil {
return nil, err
}
@@ -176,7 +176,7 @@ func generateLightClientAttackEvidence(
ConflictingBlock: &types.LightBlock{
SignedHeader: &types.SignedHeader{
Header: header,
- Commit: commit.StripExtensions(),
+ Commit: ec.ToCommit(),
},
ValidatorSet: conflictingVals,
},
diff --git a/test/e2e/tests/app_test.go b/test/e2e/tests/app_test.go
index ed041e186..20a153c1d 100644
--- a/test/e2e/tests/app_test.go
+++ b/test/e2e/tests/app_test.go
@@ -190,6 +190,7 @@ func TestApp_Tx(t *testing.T) {
func TestApp_VoteExtensions(t *testing.T) {
testNode(t, func(ctx context.Context, t *testing.T, node e2e.Node) {
+ t.Skip()
client, err := node.Client()
require.NoError(t, err)
diff --git a/types/block.go b/types/block.go
index 32a4f9a0a..f508245bb 100644
--- a/types/block.go
+++ b/types/block.go
@@ -757,22 +757,23 @@ func (ecs ExtendedCommitSig) ValidateBasic() error {
if len(ecs.Extension) > MaxVoteExtensionSize {
return fmt.Errorf("vote extension is too big (max: %d)", MaxVoteExtensionSize)
}
- if len(ecs.ExtensionSignature) == 0 {
- return errors.New("vote extension signature is missing")
- }
if len(ecs.ExtensionSignature) > MaxSignatureSize {
return fmt.Errorf("vote extension signature is too big (max: %d)", MaxSignatureSize)
}
return nil
}
- // We expect there to not be any vote extension or vote extension signature
- // on nil or absent votes.
- if len(ecs.Extension) != 0 {
- return fmt.Errorf("vote extension is present for commit sig with block ID flag %v", ecs.BlockIDFlag)
+ if len(ecs.ExtensionSignature) == 0 && len(ecs.Extension) != 0 {
+ return errors.New("vote extension signature absent on vote with extension")
}
- if len(ecs.ExtensionSignature) != 0 {
- return fmt.Errorf("vote extension signature is present for commit sig with block ID flag %v", ecs.BlockIDFlag)
+ return nil
+}
+
+// EnsureExtensions validates that a vote extensions signature is present for
+// this ExtendedCommitSig.
+func (ecs ExtendedCommitSig) EnsureExtension() error {
+ if ecs.BlockIDFlag == BlockIDFlagCommit && len(ecs.ExtensionSignature) == 0 {
+ return errors.New("vote extension data is missing")
}
return nil
}
@@ -916,6 +917,26 @@ func (commit *Commit) Hash() tmbytes.HexBytes {
return commit.hash
}
+// WrappedExtendedCommit wraps a commit as an ExtendedCommit.
+// The VoteExtension fields of the resulting value will by nil.
+// Wrapping a Commit as an ExtendedCommit is useful when an API
+// requires an ExtendedCommit wire type but does not
+// need the VoteExtension data.
+func (commit *Commit) WrappedExtendedCommit() *ExtendedCommit {
+ cs := make([]ExtendedCommitSig, len(commit.Signatures))
+ for idx, s := range commit.Signatures {
+ cs[idx] = ExtendedCommitSig{
+ CommitSig: s,
+ }
+ }
+ return &ExtendedCommit{
+ Height: commit.Height,
+ Round: commit.Round,
+ BlockID: commit.BlockID,
+ ExtendedSignatures: cs,
+ }
+}
+
// StringIndented returns a string representation of the commit.
func (commit *Commit) StringIndented(indent string) string {
if commit == nil {
@@ -1013,17 +1034,33 @@ func (ec *ExtendedCommit) Clone() *ExtendedCommit {
return &ecc
}
+// ToExtendedVoteSet constructs a VoteSet from the Commit and validator set.
+// Panics if signatures from the ExtendedCommit can't be added to the voteset.
+// Panics if any of the votes have invalid or absent vote extension data.
+// Inverse of VoteSet.MakeExtendedCommit().
+func (ec *ExtendedCommit) ToExtendedVoteSet(chainID string, vals *ValidatorSet) *VoteSet {
+ voteSet := NewExtendedVoteSet(chainID, ec.Height, ec.Round, tmproto.PrecommitType, vals)
+ ec.addSigsToVoteSet(voteSet)
+ return voteSet
+}
+
// ToVoteSet constructs a VoteSet from the Commit and validator set.
-// Panics if signatures from the commit can't be added to the voteset.
+// Panics if signatures from the ExtendedCommit can't be added to the voteset.
// Inverse of VoteSet.MakeExtendedCommit().
func (ec *ExtendedCommit) ToVoteSet(chainID string, vals *ValidatorSet) *VoteSet {
voteSet := NewVoteSet(chainID, ec.Height, ec.Round, tmproto.PrecommitType, vals)
+ ec.addSigsToVoteSet(voteSet)
+ return voteSet
+}
+
+// addSigsToVoteSet adds all of the signature to voteSet.
+func (ec *ExtendedCommit) addSigsToVoteSet(voteSet *VoteSet) {
for idx, ecs := range ec.ExtendedSignatures {
if ecs.BlockIDFlag == BlockIDFlagAbsent {
continue // OK, some precommits can be missing.
}
vote := ec.GetExtendedVote(int32(idx))
- if err := vote.ValidateWithExtension(); err != nil {
+ if err := vote.ValidateBasic(); err != nil {
panic(fmt.Errorf("failed to validate vote reconstructed from LastCommit: %w", err))
}
added, err := voteSet.AddVote(vote)
@@ -1031,12 +1068,58 @@ func (ec *ExtendedCommit) ToVoteSet(chainID string, vals *ValidatorSet) *VoteSet
panic(fmt.Errorf("failed to reconstruct vote set from extended commit: %w", err))
}
}
+}
+
+// ToVoteSet constructs a VoteSet from the Commit and validator set.
+// Panics if signatures from the commit can't be added to the voteset.
+// Inverse of VoteSet.MakeCommit().
+func (commit *Commit) ToVoteSet(chainID string, vals *ValidatorSet) *VoteSet {
+ voteSet := NewVoteSet(chainID, commit.Height, commit.Round, tmproto.PrecommitType, vals)
+ for idx, cs := range commit.Signatures {
+ if cs.BlockIDFlag == BlockIDFlagAbsent {
+ continue // OK, some precommits can be missing.
+ }
+ vote := commit.GetVote(int32(idx))
+ if err := vote.ValidateBasic(); err != nil {
+ panic(fmt.Errorf("failed to validate vote reconstructed from commit: %w", err))
+ }
+ added, err := voteSet.AddVote(vote)
+ if !added || err != nil {
+ panic(fmt.Errorf("failed to reconstruct vote set from commit: %w", err))
+ }
+ }
return voteSet
}
-// StripExtensions converts an ExtendedCommit to a Commit by removing all vote
+// EnsureExtensions validates that a vote extensions signature is present for
+// every ExtendedCommitSig in the ExtendedCommit.
+func (ec *ExtendedCommit) EnsureExtensions() error {
+ for _, ecs := range ec.ExtendedSignatures {
+ if err := ecs.EnsureExtension(); err != nil {
+ return err
+ }
+ }
+ return nil
+}
+
+// StripExtensions removes all VoteExtension data from an ExtendedCommit. This
+// is useful when dealing with an ExendedCommit but vote extension data is
+// expected to be absent.
+func (ec *ExtendedCommit) StripExtensions() bool {
+ stripped := false
+ for idx := range ec.ExtendedSignatures {
+ if len(ec.ExtendedSignatures[idx].Extension) > 0 || len(ec.ExtendedSignatures[idx].ExtensionSignature) > 0 {
+ stripped = true
+ }
+ ec.ExtendedSignatures[idx].Extension = nil
+ ec.ExtendedSignatures[idx].ExtensionSignature = nil
+ }
+ return stripped
+}
+
+// ToCommit converts an ExtendedCommit to a Commit by removing all vote
// extension-related fields.
-func (ec *ExtendedCommit) StripExtensions() *Commit {
+func (ec *ExtendedCommit) ToCommit() *Commit {
cs := make([]CommitSig, len(ec.ExtendedSignatures))
for idx, ecs := range ec.ExtendedSignatures {
cs[idx] = ecs.CommitSig
diff --git a/types/block_test.go b/types/block_test.go
index 09a8b602e..4c3a74d8f 100644
--- a/types/block_test.go
+++ b/types/block_test.go
@@ -49,7 +49,7 @@ func TestBlockAddEvidence(t *testing.T) {
require.NoError(t, err)
evList := []Evidence{ev}
- block := MakeBlock(h, txs, extCommit.StripExtensions(), evList)
+ block := MakeBlock(h, txs, extCommit.ToCommit(), evList)
require.NotNil(t, block)
require.Equal(t, 1, len(block.Evidence))
require.NotNil(t, block.EvidenceHash)
@@ -68,7 +68,7 @@ func TestBlockValidateBasic(t *testing.T) {
voteSet, valSet, vals := randVoteSet(ctx, t, h-1, 1, tmproto.PrecommitType, 10, 1)
extCommit, err := makeExtCommit(ctx, lastID, h-1, 1, voteSet, vals, time.Now())
require.NoError(t, err)
- commit := extCommit.StripExtensions()
+ commit := extCommit.ToCommit()
ev, err := NewMockDuplicateVoteEvidenceWithValidator(ctx, h, time.Now(), vals[0], "block-test-chain")
require.NoError(t, err)
@@ -163,7 +163,7 @@ func TestBlockMakePartSetWithEvidence(t *testing.T) {
require.NoError(t, err)
evList := []Evidence{ev}
- partSet, err := MakeBlock(h, []Tx{Tx("Hello World")}, extCommit.StripExtensions(), evList).MakePartSet(512)
+ partSet, err := MakeBlock(h, []Tx{Tx("Hello World")}, extCommit.ToCommit(), evList).MakePartSet(512)
require.NoError(t, err)
assert.NotNil(t, partSet)
@@ -187,7 +187,7 @@ func TestBlockHashesTo(t *testing.T) {
require.NoError(t, err)
evList := []Evidence{ev}
- block := MakeBlock(h, []Tx{Tx("Hello World")}, extCommit.StripExtensions(), evList)
+ block := MakeBlock(h, []Tx{Tx("Hello World")}, extCommit.ToCommit(), evList)
block.ValidatorsHash = valSet.Hash()
assert.False(t, block.HashesTo([]byte{}))
assert.False(t, block.HashesTo([]byte("something else")))
@@ -483,7 +483,7 @@ func randCommit(ctx context.Context, t *testing.T, now time.Time) *Commit {
require.NoError(t, err)
- return commit.StripExtensions()
+ return commit.ToCommit()
}
func hexBytesFromString(t *testing.T, s string) bytes.HexBytes {
@@ -556,33 +556,138 @@ func TestBlockMaxDataBytesNoEvidence(t *testing.T) {
}
}
+// TestVoteSetToExtendedCommit tests that the extended commit produced from a
+// vote set contains the same vote information as the vote set. The test ensures
+// that the MakeExtendedCommit method behaves as expected, whether vote extensions
+// are present in the original votes or not.
+func TestVoteSetToExtendedCommit(t *testing.T) {
+ for _, testCase := range []struct {
+ name string
+ includeExtension bool
+ }{
+ {
+ name: "no extensions",
+ includeExtension: false,
+ },
+ {
+ name: "with extensions",
+ includeExtension: true,
+ },
+ } {
+
+ t.Run(testCase.name, func(t *testing.T) {
+ blockID := makeBlockIDRandom()
+ ctx, cancel := context.WithCancel(context.Background())
+ defer cancel()
+
+ valSet, vals := randValidatorPrivValSet(ctx, t, 10, 1)
+ var voteSet *VoteSet
+ if testCase.includeExtension {
+ voteSet = NewExtendedVoteSet("test_chain_id", 3, 1, tmproto.PrecommitType, valSet)
+ } else {
+ voteSet = NewVoteSet("test_chain_id", 3, 1, tmproto.PrecommitType, valSet)
+ }
+ for i := 0; i < len(vals); i++ {
+ pubKey, err := vals[i].GetPubKey(ctx)
+ require.NoError(t, err)
+ vote := &Vote{
+ ValidatorAddress: pubKey.Address(),
+ ValidatorIndex: int32(i),
+ Height: 3,
+ Round: 1,
+ Type: tmproto.PrecommitType,
+ BlockID: blockID,
+ Timestamp: time.Now(),
+ }
+ v := vote.ToProto()
+ err = vals[i].SignVote(ctx, voteSet.ChainID(), v)
+ require.NoError(t, err)
+ vote.Signature = v.Signature
+ if testCase.includeExtension {
+ vote.ExtensionSignature = v.ExtensionSignature
+ }
+ added, err := voteSet.AddVote(vote)
+ require.NoError(t, err)
+ require.True(t, added)
+ }
+ ec := voteSet.MakeExtendedCommit()
+
+ for i := int32(0); int(i) < len(vals); i++ {
+ vote1 := voteSet.GetByIndex(i)
+ vote2 := ec.GetExtendedVote(i)
+
+ vote1bz, err := vote1.ToProto().Marshal()
+ require.NoError(t, err)
+ vote2bz, err := vote2.ToProto().Marshal()
+ require.NoError(t, err)
+ assert.Equal(t, vote1bz, vote2bz)
+ }
+ })
+ }
+}
+
+// TestExtendedCommitToVoteSet tests that the vote set produced from an extended commit
+// contains the same vote information as the extended commit. The test ensures
+// that the ToVoteSet method behaves as expected, whether vote extensions
+// are present in the original votes or not.
func TestExtendedCommitToVoteSet(t *testing.T) {
- lastID := makeBlockIDRandom()
- h := int64(3)
+ for _, testCase := range []struct {
+ name string
+ includeExtension bool
+ }{
+ {
+ name: "no extensions",
+ includeExtension: false,
+ },
+ {
+ name: "with extensions",
+ includeExtension: true,
+ },
+ } {
+ t.Run(testCase.name, func(t *testing.T) {
+ lastID := makeBlockIDRandom()
+ h := int64(3)
- ctx, cancel := context.WithCancel(context.Background())
- defer cancel()
+ ctx, cancel := context.WithCancel(context.Background())
+ defer cancel()
- voteSet, valSet, vals := randVoteSet(ctx, t, h-1, 1, tmproto.PrecommitType, 10, 1)
- extCommit, err := makeExtCommit(ctx, lastID, h-1, 1, voteSet, vals, time.Now())
- assert.NoError(t, err)
+ voteSet, valSet, vals := randVoteSet(ctx, t, h-1, 1, tmproto.PrecommitType, 10, 1)
+ extCommit, err := makeExtCommit(ctx, lastID, h-1, 1, voteSet, vals, time.Now())
+ assert.NoError(t, err)
- chainID := voteSet.ChainID()
- voteSet2 := extCommit.ToVoteSet(chainID, valSet)
+ if !testCase.includeExtension {
+ for i := 0; i < len(vals); i++ {
+ v := voteSet.GetByIndex(int32(i))
+ v.Extension = nil
+ v.ExtensionSignature = nil
+ extCommit.ExtendedSignatures[i].Extension = nil
+ extCommit.ExtendedSignatures[i].ExtensionSignature = nil
+ }
+ }
- for i := int32(0); int(i) < len(vals); i++ {
- vote1 := voteSet.GetByIndex(i)
- vote2 := voteSet2.GetByIndex(i)
- vote3 := extCommit.GetExtendedVote(i)
+ chainID := voteSet.ChainID()
+ var voteSet2 *VoteSet
+ if testCase.includeExtension {
+ voteSet2 = extCommit.ToExtendedVoteSet(chainID, valSet)
+ } else {
+ voteSet2 = extCommit.ToVoteSet(chainID, valSet)
+ }
- vote1bz, err := vote1.ToProto().Marshal()
- require.NoError(t, err)
- vote2bz, err := vote2.ToProto().Marshal()
- require.NoError(t, err)
- vote3bz, err := vote3.ToProto().Marshal()
- require.NoError(t, err)
- assert.Equal(t, vote1bz, vote2bz)
- assert.Equal(t, vote1bz, vote3bz)
+ for i := int32(0); int(i) < len(vals); i++ {
+ vote1 := voteSet.GetByIndex(i)
+ vote2 := voteSet2.GetByIndex(i)
+ vote3 := extCommit.GetExtendedVote(i)
+
+ vote1bz, err := vote1.ToProto().Marshal()
+ require.NoError(t, err)
+ vote2bz, err := vote2.ToProto().Marshal()
+ require.NoError(t, err)
+ vote3bz, err := vote3.ToProto().Marshal()
+ require.NoError(t, err)
+ assert.Equal(t, vote1bz, vote2bz)
+ assert.Equal(t, vote1bz, vote3bz)
+ }
+ })
}
}
@@ -637,7 +742,7 @@ func TestCommitToVoteSetWithVotesForNilBlock(t *testing.T) {
if tc.valid {
extCommit := voteSet.MakeExtendedCommit() // panics without > 2/3 valid votes
assert.NotNil(t, extCommit)
- err := valSet.VerifyCommit(voteSet.ChainID(), blockID, height-1, extCommit.StripExtensions())
+ err := valSet.VerifyCommit(voteSet.ChainID(), blockID, height-1, extCommit.ToCommit())
assert.NoError(t, err)
} else {
assert.Panics(t, func() { voteSet.MakeExtendedCommit() })
diff --git a/types/evidence_test.go b/types/evidence_test.go
index 8b06a6218..d014a3ecc 100644
--- a/types/evidence_test.go
+++ b/types/evidence_test.go
@@ -155,7 +155,7 @@ func TestLightClientAttackEvidenceBasic(t *testing.T) {
blockID := makeBlockID(crypto.Checksum([]byte("blockhash")), math.MaxInt32, crypto.Checksum([]byte("partshash")))
extCommit, err := makeExtCommit(ctx, blockID, height, 1, voteSet, privVals, defaultVoteTime)
require.NoError(t, err)
- commit := extCommit.StripExtensions()
+ commit := extCommit.ToCommit()
lcae := &LightClientAttackEvidence{
ConflictingBlock: &LightBlock{
@@ -221,7 +221,7 @@ func TestLightClientAttackEvidenceValidation(t *testing.T) {
blockID := makeBlockID(header.Hash(), math.MaxInt32, crypto.Checksum([]byte("partshash")))
extCommit, err := makeExtCommit(ctx, blockID, height, 1, voteSet, privVals, time.Now())
require.NoError(t, err)
- commit := extCommit.StripExtensions()
+ commit := extCommit.ToCommit()
lcae := &LightClientAttackEvidence{
ConflictingBlock: &LightBlock{
@@ -434,7 +434,7 @@ func TestEvidenceVectors(t *testing.T) {
ConflictingBlock: &LightBlock{
SignedHeader: &SignedHeader{
Header: header,
- Commit: extCommit.StripExtensions(),
+ Commit: extCommit.ToCommit(),
},
ValidatorSet: valSet,
},
diff --git a/types/params.go b/types/params.go
index e8ee6fcdf..3b5e9a250 100644
--- a/types/params.go
+++ b/types/params.go
@@ -43,6 +43,7 @@ type ConsensusParams struct {
Version VersionParams `json:"version"`
Synchrony SynchronyParams `json:"synchrony"`
Timeout TimeoutParams `json:"timeout"`
+ ABCI ABCIParams `json:"abci"`
}
// HashedParams is a subset of ConsensusParams.
@@ -96,6 +97,21 @@ type TimeoutParams struct {
BypassCommitTimeout bool `json:"bypass_commit_timeout"`
}
+// ABCIParams configure ABCI functionality specific to the Application Blockchain
+// Interface.
+type ABCIParams struct {
+ VoteExtensionsEnableHeight int64 `json:"vote_extensions_enable_height"`
+}
+
+// VoteExtensionsEnabled returns true if vote extensions are enabled at height h
+// and false otherwise.
+func (a ABCIParams) VoteExtensionsEnabled(h int64) bool {
+ if a.VoteExtensionsEnableHeight == 0 {
+ return false
+ }
+ return a.VoteExtensionsEnableHeight <= h
+}
+
// DefaultConsensusParams returns a default ConsensusParams.
func DefaultConsensusParams() *ConsensusParams {
return &ConsensusParams{
@@ -105,6 +121,7 @@ func DefaultConsensusParams() *ConsensusParams {
Version: DefaultVersionParams(),
Synchrony: DefaultSynchronyParams(),
Timeout: DefaultTimeoutParams(),
+ ABCI: DefaultABCIParams(),
}
}
@@ -176,6 +193,13 @@ func DefaultTimeoutParams() TimeoutParams {
}
}
+func DefaultABCIParams() ABCIParams {
+ return ABCIParams{
+ // When set to 0, vote extensions are not required.
+ VoteExtensionsEnableHeight: 0,
+ }
+}
+
// TimeoutParamsOrDefaults returns the SynchronyParams, filling in any zero values
// with the Tendermint defined default values.
func (t TimeoutParams) TimeoutParamsOrDefaults() TimeoutParams {
diff --git a/types/validation_test.go b/types/validation_test.go
index f63c34450..c28f63000 100644
--- a/types/validation_test.go
+++ b/types/validation_test.go
@@ -153,7 +153,7 @@ func TestValidatorSet_VerifyCommit_CheckAllSignatures(t *testing.T) {
voteSet, valSet, vals := randVoteSet(ctx, t, h, 0, tmproto.PrecommitType, 4, 10)
extCommit, err := makeExtCommit(ctx, blockID, h, 0, voteSet, vals, time.Now())
require.NoError(t, err)
- commit := extCommit.StripExtensions()
+ commit := extCommit.ToCommit()
require.NoError(t, valSet.VerifyCommit(chainID, blockID, h, commit))
@@ -184,7 +184,7 @@ func TestValidatorSet_VerifyCommitLight_ReturnsAsSoonAsMajorityOfVotingPowerSign
voteSet, valSet, vals := randVoteSet(ctx, t, h, 0, tmproto.PrecommitType, 4, 10)
extCommit, err := makeExtCommit(ctx, blockID, h, 0, voteSet, vals, time.Now())
require.NoError(t, err)
- commit := extCommit.StripExtensions()
+ commit := extCommit.ToCommit()
require.NoError(t, valSet.VerifyCommit(chainID, blockID, h, commit))
@@ -212,7 +212,7 @@ func TestValidatorSet_VerifyCommitLightTrusting_ReturnsAsSoonAsTrustLevelOfVotin
voteSet, valSet, vals := randVoteSet(ctx, t, h, 0, tmproto.PrecommitType, 4, 10)
extCommit, err := makeExtCommit(ctx, blockID, h, 0, voteSet, vals, time.Now())
require.NoError(t, err)
- commit := extCommit.StripExtensions()
+ commit := extCommit.ToCommit()
require.NoError(t, valSet.VerifyCommit(chainID, blockID, h, commit))
@@ -239,7 +239,7 @@ func TestValidatorSet_VerifyCommitLightTrusting(t *testing.T) {
newValSet, _ = randValidatorPrivValSet(ctx, t, 2, 1)
)
require.NoError(t, err)
- commit := extCommit.StripExtensions()
+ commit := extCommit.ToCommit()
testCases := []struct {
valSet *ValidatorSet
@@ -284,7 +284,7 @@ func TestValidatorSet_VerifyCommitLightTrustingErrorsOnOverflow(t *testing.T) {
)
require.NoError(t, err)
- err = valSet.VerifyCommitLightTrusting("test_chain_id", extCommit.StripExtensions(),
+ err = valSet.VerifyCommitLightTrusting("test_chain_id", extCommit.ToCommit(),
tmmath.Fraction{Numerator: 25, Denominator: 55})
if assert.Error(t, err) {
assert.Contains(t, err.Error(), "int64 overflow")
diff --git a/types/validator_set_test.go b/types/validator_set_test.go
index 8b5846da9..81e81919d 100644
--- a/types/validator_set_test.go
+++ b/types/validator_set_test.go
@@ -1541,7 +1541,7 @@ func BenchmarkValidatorSet_VerifyCommit_Ed25519(b *testing.B) { // nolint
// create a commit with n validators
extCommit, err := makeExtCommit(ctx, blockID, h, 0, voteSet, vals, time.Now())
require.NoError(b, err)
- commit := extCommit.StripExtensions()
+ commit := extCommit.ToCommit()
for i := 0; i < b.N/n; i++ {
err = valSet.VerifyCommit(chainID, blockID, h, commit)
@@ -1570,7 +1570,7 @@ func BenchmarkValidatorSet_VerifyCommitLight_Ed25519(b *testing.B) { // nolint
// create a commit with n validators
extCommit, err := makeExtCommit(ctx, blockID, h, 0, voteSet, vals, time.Now())
require.NoError(b, err)
- commit := extCommit.StripExtensions()
+ commit := extCommit.ToCommit()
for i := 0; i < b.N/n; i++ {
err = valSet.VerifyCommitLight(chainID, blockID, h, commit)
@@ -1598,7 +1598,7 @@ func BenchmarkValidatorSet_VerifyCommitLightTrusting_Ed25519(b *testing.B) {
// create a commit with n validators
extCommit, err := makeExtCommit(ctx, blockID, h, 0, voteSet, vals, time.Now())
require.NoError(b, err)
- commit := extCommit.StripExtensions()
+ commit := extCommit.ToCommit()
for i := 0; i < b.N/n; i++ {
err = valSet.VerifyCommitLightTrusting(chainID, commit, tmmath.Fraction{Numerator: 1, Denominator: 3})
diff --git a/types/vote.go b/types/vote.go
index 446de130a..f7006b8cd 100644
--- a/types/vote.go
+++ b/types/vote.go
@@ -27,7 +27,7 @@ var (
ErrVoteInvalidBlockHash = errors.New("invalid block hash")
ErrVoteNonDeterministicSignature = errors.New("non-deterministic signature")
ErrVoteNil = errors.New("nil vote")
- ErrVoteInvalidExtension = errors.New("invalid vote extension")
+ ErrVoteExtensionAbsent = errors.New("vote extension absent")
)
type ErrVoteConflictingVotes struct {
@@ -112,6 +112,16 @@ func (vote *Vote) CommitSig() CommitSig {
}
}
+// StripExtension removes any extension data from the vote. Useful if the
+// chain has not enabled vote extensions.
+// Returns true if extension data was present before stripping and false otherwise.
+func (vote *Vote) StripExtension() bool {
+ stripped := len(vote.Extension) > 0 || len(vote.ExtensionSignature) > 0
+ vote.Extension = nil
+ vote.ExtensionSignature = nil
+ return stripped
+}
+
// ExtendedCommitSig attempts to construct an ExtendedCommitSig from this vote.
// Panics if either the vote extension signature is missing or if the block ID
// is not either empty or complete.
@@ -120,13 +130,8 @@ func (vote *Vote) ExtendedCommitSig() ExtendedCommitSig {
return NewExtendedCommitSigAbsent()
}
- cs := vote.CommitSig()
- if vote.BlockID.IsComplete() && len(vote.ExtensionSignature) == 0 {
- panic(fmt.Sprintf("Invalid vote %v - BlockID is complete but missing vote extension signature", vote))
- }
-
return ExtendedCommitSig{
- CommitSig: cs,
+ CommitSig: vote.CommitSig(),
Extension: vote.Extension,
ExtensionSignature: vote.ExtensionSignature,
}
@@ -230,11 +235,11 @@ func (vote *Vote) Verify(chainID string, pubKey crypto.PubKey) error {
return err
}
-// VerifyWithExtension performs the same verification as Verify, but
+// VerifyVoteAndExtension performs the same verification as Verify, but
// additionally checks whether the vote extension signature corresponds to the
// given chain ID and public key. We only verify vote extension signatures for
// precommits.
-func (vote *Vote) VerifyWithExtension(chainID string, pubKey crypto.PubKey) error {
+func (vote *Vote) VerifyVoteAndExtension(chainID string, pubKey crypto.PubKey) error {
v, err := vote.verifyAndReturnProto(chainID, pubKey)
if err != nil {
return err
@@ -249,6 +254,20 @@ func (vote *Vote) VerifyWithExtension(chainID string, pubKey crypto.PubKey) erro
return nil
}
+// VerifyExtension checks whether the vote extension signature corresponds to the
+// given chain ID and public key.
+func (vote *Vote) VerifyExtension(chainID string, pubKey crypto.PubKey) error {
+ if vote.Type != tmproto.PrecommitType || vote.BlockID.IsNil() {
+ return nil
+ }
+ v := vote.ToProto()
+ extSignBytes := VoteExtensionSignBytes(chainID, v)
+ if !pubKey.VerifySignature(extSignBytes, vote.ExtensionSignature) {
+ return ErrVoteInvalidSignature
+ }
+ return nil
+}
+
// ValidateBasic checks whether the vote is well-formed. It does not, however,
// check vote extensions - for vote validation with vote extension validation,
// use ValidateWithExtension.
@@ -306,30 +325,34 @@ func (vote *Vote) ValidateBasic() error {
}
}
- return nil
-}
-
-// ValidateWithExtension performs the same validations as ValidateBasic, but
-// additionally checks whether a vote extension signature is present. This
-// function is used in places where vote extension signatures are expected.
-func (vote *Vote) ValidateWithExtension() error {
- if err := vote.ValidateBasic(); err != nil {
- return err
- }
-
- // We should always see vote extension signatures in non-nil precommits
if vote.Type == tmproto.PrecommitType && !vote.BlockID.IsNil() {
- if len(vote.ExtensionSignature) == 0 {
- return errors.New("vote extension signature is missing")
- }
if len(vote.ExtensionSignature) > MaxSignatureSize {
return fmt.Errorf("vote extension signature is too big (max: %d)", MaxSignatureSize)
}
+ if len(vote.ExtensionSignature) == 0 && len(vote.Extension) != 0 {
+ return fmt.Errorf("vote extension signature absent on vote with extension")
+ }
}
return nil
}
+// EnsureExtension checks for the presence of extensions signature data
+// on precommit vote types.
+func (vote *Vote) EnsureExtension() error {
+ // We should always see vote extension signatures in non-nil precommits
+ if vote.Type != tmproto.PrecommitType {
+ return nil
+ }
+ if vote.BlockID.IsNil() {
+ return nil
+ }
+ if len(vote.ExtensionSignature) > 0 {
+ return nil
+ }
+ return ErrVoteExtensionAbsent
+}
+
// ToProto converts the handwritten type to proto generated type
// return type, nil if everything converts safely, otherwise nil, error
func (vote *Vote) ToProto() *tmproto.Vote {
diff --git a/types/vote_set.go b/types/vote_set.go
index 224d4e4f8..6d83ac85d 100644
--- a/types/vote_set.go
+++ b/types/vote_set.go
@@ -3,6 +3,7 @@ package types
import (
"bytes"
"encoding/json"
+ "errors"
"fmt"
"strings"
"sync"
@@ -53,11 +54,12 @@ const (
NOTE: Assumes that the sum total of voting power does not exceed MaxUInt64.
*/
type VoteSet struct {
- chainID string
- height int64
- round int32
- signedMsgType tmproto.SignedMsgType
- valSet *ValidatorSet
+ chainID string
+ height int64
+ round int32
+ signedMsgType tmproto.SignedMsgType
+ valSet *ValidatorSet
+ extensionsEnabled bool
mtx sync.Mutex
votesBitArray *bits.BitArray
@@ -68,7 +70,8 @@ type VoteSet struct {
peerMaj23s map[string]BlockID // Maj23 for each peer
}
-// Constructs a new VoteSet struct used to accumulate votes for given height/round.
+// NewVoteSet instantiates all fields of a new vote set. This constructor requires
+// that no vote extension data be present on the votes that are added to the set.
func NewVoteSet(chainID string, height int64, round int32,
signedMsgType tmproto.SignedMsgType, valSet *ValidatorSet) *VoteSet {
if height == 0 {
@@ -89,6 +92,16 @@ func NewVoteSet(chainID string, height int64, round int32,
}
}
+// NewExtendedVoteSet constructs a vote set with additional vote verification logic.
+// The VoteSet constructed with NewExtendedVoteSet verifies the vote extension
+// data for every vote added to the set.
+func NewExtendedVoteSet(chainID string, height int64, round int32,
+ signedMsgType tmproto.SignedMsgType, valSet *ValidatorSet) *VoteSet {
+ vs := NewVoteSet(chainID, height, round, signedMsgType, valSet)
+ vs.extensionsEnabled = true
+ return vs
+}
+
func (voteSet *VoteSet) ChainID() string {
return voteSet.chainID
}
@@ -194,8 +207,17 @@ func (voteSet *VoteSet) addVote(vote *Vote) (added bool, err error) {
}
// Check signature.
- if err := vote.VerifyWithExtension(voteSet.chainID, val.PubKey); err != nil {
- return false, fmt.Errorf("failed to verify vote with ChainID %s and PubKey %s: %w", voteSet.chainID, val.PubKey, err)
+ if voteSet.extensionsEnabled {
+ if err := vote.VerifyVoteAndExtension(voteSet.chainID, val.PubKey); err != nil {
+ return false, fmt.Errorf("failed to verify vote with ChainID %s and PubKey %s: %w", voteSet.chainID, val.PubKey, err)
+ }
+ } else {
+ if err := vote.Verify(voteSet.chainID, val.PubKey); err != nil {
+ return false, fmt.Errorf("failed to verify vote with ChainID %s and PubKey %s: %w", voteSet.chainID, val.PubKey, err)
+ }
+ if len(vote.ExtensionSignature) > 0 || len(vote.Extension) > 0 {
+ return false, errors.New("unexpected vote extension data present in vote")
+ }
}
// Add vote and get conflicting vote if any.
diff --git a/types/vote_set_test.go b/types/vote_set_test.go
index 8d166d508..e35da7491 100644
--- a/types/vote_set_test.go
+++ b/types/vote_set_test.go
@@ -498,6 +498,92 @@ func TestVoteSet_MakeCommit(t *testing.T) {
}
}
+// TestVoteSet_VoteExtensionsEnabled tests that the vote set correctly validates
+// vote extensions data when either required or not required.
+func TestVoteSet_VoteExtensionsEnabled(t *testing.T) {
+ for _, tc := range []struct {
+ name string
+ requireExtensions bool
+ addExtension bool
+ exepectError bool
+ }{
+ {
+ name: "no extension but expected",
+ requireExtensions: true,
+ addExtension: false,
+ exepectError: true,
+ },
+ {
+ name: "invalid extensions but not expected",
+ requireExtensions: true,
+ addExtension: false,
+ exepectError: true,
+ },
+ {
+ name: "no extension and not expected",
+ requireExtensions: false,
+ addExtension: false,
+ exepectError: false,
+ },
+ {
+ name: "extension and expected",
+ requireExtensions: true,
+ addExtension: true,
+ exepectError: false,
+ },
+ } {
+ t.Run(tc.name, func(t *testing.T) {
+ ctx, cancel := context.WithCancel(context.Background())
+ defer cancel()
+
+ height, round := int64(1), int32(0)
+ valSet, privValidators := randValidatorPrivValSet(ctx, t, 5, 10)
+ var voteSet *VoteSet
+ if tc.requireExtensions {
+ voteSet = NewExtendedVoteSet("test_chain_id", height, round, tmproto.PrecommitType, valSet)
+ } else {
+ voteSet = NewVoteSet("test_chain_id", height, round, tmproto.PrecommitType, valSet)
+ }
+
+ val0 := privValidators[0]
+
+ val0p, err := val0.GetPubKey(ctx)
+ require.NoError(t, err)
+ val0Addr := val0p.Address()
+ blockHash := crypto.CRandBytes(32)
+ blockPartsTotal := uint32(123)
+ blockPartSetHeader := PartSetHeader{blockPartsTotal, crypto.CRandBytes(32)}
+
+ vote := &Vote{
+ ValidatorAddress: val0Addr,
+ ValidatorIndex: 0,
+ Height: height,
+ Round: round,
+ Type: tmproto.PrecommitType,
+ Timestamp: tmtime.Now(),
+ BlockID: BlockID{blockHash, blockPartSetHeader},
+ }
+ v := vote.ToProto()
+ err = val0.SignVote(ctx, voteSet.ChainID(), v)
+ require.NoError(t, err)
+ vote.Signature = v.Signature
+
+ if tc.addExtension {
+ vote.ExtensionSignature = v.ExtensionSignature
+ }
+
+ added, err := voteSet.AddVote(vote)
+ if tc.exepectError {
+ require.Error(t, err)
+ require.False(t, added)
+ } else {
+ require.NoError(t, err)
+ require.True(t, added)
+ }
+ })
+ }
+}
+
// NOTE: privValidators are in order
func randVoteSet(
ctx context.Context,
@@ -510,7 +596,7 @@ func randVoteSet(
) (*VoteSet, *ValidatorSet, []PrivValidator) {
t.Helper()
valSet, privValidators := randValidatorPrivValSet(ctx, t, numValidators, votingPower)
- return NewVoteSet("test_chain_id", height, round, signedMsgType, valSet), valSet, privValidators
+ return NewExtendedVoteSet("test_chain_id", height, round, signedMsgType, valSet), valSet, privValidators
}
func deterministicVoteSet(
@@ -523,7 +609,7 @@ func deterministicVoteSet(
) (*VoteSet, *ValidatorSet, []PrivValidator) {
t.Helper()
valSet, privValidators := deterministicValidatorSet(ctx, t)
- return NewVoteSet("test_chain_id", height, round, signedMsgType, valSet), valSet, privValidators
+ return NewExtendedVoteSet("test_chain_id", height, round, signedMsgType, valSet), valSet, privValidators
}
func randValidatorPrivValSet(ctx context.Context, t testing.TB, numValidators int, votingPower int64) (*ValidatorSet, []PrivValidator) {
diff --git a/types/vote_test.go b/types/vote_test.go
index 70cd91381..d0819d7c4 100644
--- a/types/vote_test.go
+++ b/types/vote_test.go
@@ -267,7 +267,7 @@ func TestVoteExtension(t *testing.T) {
if tc.includeSignature {
vote.ExtensionSignature = v.ExtensionSignature
}
- err = vote.VerifyWithExtension("test_chain_id", pk)
+ err = vote.VerifyExtension("test_chain_id", pk)
if tc.expectError {
require.Error(t, err)
} else {
@@ -361,7 +361,7 @@ func TestValidVotes(t *testing.T) {
signVote(ctx, t, privVal, "test_chain_id", tc.vote)
tc.malleateVote(tc.vote)
require.NoError(t, tc.vote.ValidateBasic(), "ValidateBasic for %s", tc.name)
- require.NoError(t, tc.vote.ValidateWithExtension(), "ValidateWithExtension for %s", tc.name)
+ require.NoError(t, tc.vote.EnsureExtension(), "EnsureExtension for %s", tc.name)
}
}
@@ -387,13 +387,13 @@ func TestInvalidVotes(t *testing.T) {
signVote(ctx, t, privVal, "test_chain_id", prevote)
tc.malleateVote(prevote)
require.Error(t, prevote.ValidateBasic(), "ValidateBasic for %s in invalid prevote", tc.name)
- require.Error(t, prevote.ValidateWithExtension(), "ValidateWithExtension for %s in invalid prevote", tc.name)
+ require.NoError(t, prevote.EnsureExtension(), "EnsureExtension for %s in invalid prevote", tc.name)
precommit := examplePrecommit(t)
signVote(ctx, t, privVal, "test_chain_id", precommit)
tc.malleateVote(precommit)
require.Error(t, precommit.ValidateBasic(), "ValidateBasic for %s in invalid precommit", tc.name)
- require.Error(t, precommit.ValidateWithExtension(), "ValidateWithExtension for %s in invalid precommit", tc.name)
+ require.NoError(t, precommit.EnsureExtension(), "EnsureExtension for %s in invalid precommit", tc.name)
}
}
@@ -414,7 +414,7 @@ func TestInvalidPrevotes(t *testing.T) {
signVote(ctx, t, privVal, "test_chain_id", prevote)
tc.malleateVote(prevote)
require.Error(t, prevote.ValidateBasic(), "ValidateBasic for %s", tc.name)
- require.Error(t, prevote.ValidateWithExtension(), "ValidateWithExtension for %s", tc.name)
+ require.NoError(t, prevote.EnsureExtension(), "EnsureExtension for %s", tc.name)
}
}
@@ -431,18 +431,44 @@ func TestInvalidPrecommitExtensions(t *testing.T) {
v.Extension = []byte("extension")
v.ExtensionSignature = nil
}},
- // TODO(thane): Re-enable once https://github.com/tendermint/tendermint/issues/8272 is resolved
- //{"missing vote extension signature", func(v *Vote) { v.ExtensionSignature = nil }},
{"oversized vote extension signature", func(v *Vote) { v.ExtensionSignature = make([]byte, MaxSignatureSize+1) }},
}
for _, tc := range testCases {
precommit := examplePrecommit(t)
signVote(ctx, t, privVal, "test_chain_id", precommit)
tc.malleateVote(precommit)
- // We don't expect an error from ValidateBasic, because it doesn't
- // handle vote extensions.
- require.NoError(t, precommit.ValidateBasic(), "ValidateBasic for %s", tc.name)
- require.Error(t, precommit.ValidateWithExtension(), "ValidateWithExtension for %s", tc.name)
+ // ValidateBasic ensures that vote extensions, if present, are well formed
+ require.Error(t, precommit.ValidateBasic(), "ValidateBasic for %s", tc.name)
+ }
+}
+
+func TestEnsureVoteExtension(t *testing.T) {
+ ctx, cancel := context.WithCancel(context.Background())
+ defer cancel()
+ privVal := NewMockPV()
+
+ testCases := []struct {
+ name string
+ malleateVote func(*Vote)
+ expectError bool
+ }{
+ {"vote extension signature absent", func(v *Vote) {
+ v.Extension = nil
+ v.ExtensionSignature = nil
+ }, true},
+ {"vote extension signature present", func(v *Vote) {
+ v.ExtensionSignature = []byte("extension signature")
+ }, false},
+ }
+ for _, tc := range testCases {
+ precommit := examplePrecommit(t)
+ signVote(ctx, t, privVal, "test_chain_id", precommit)
+ tc.malleateVote(precommit)
+ if tc.expectError {
+ require.Error(t, precommit.EnsureExtension(), "EnsureExtension for %s", tc.name)
+ } else {
+ require.NoError(t, precommit.EnsureExtension(), "EnsureExtension for %s", tc.name)
+ }
}
}
@@ -497,7 +523,7 @@ func getSampleCommit(ctx context.Context, t testing.TB) *Commit {
require.NoError(t, err)
- return commit.StripExtensions()
+ return commit.ToCommit()
}
func BenchmarkVoteSignBytes(b *testing.B) {