abci++: Sync implementation and spec for vote extensions (#8141)

* Refactor so building and linting works This is the first step towards implementing vote extensions: generating the relevant proto stubs and getting the build and linter to pass. Signed-off-by: Thane Thomson <connect@thanethomson.com> * Fix typo Signed-off-by: Thane Thomson <connect@thanethomson.com> * Better describe method given vote extensions Signed-off-by: Thane Thomson <connect@thanethomson.com> * Fix types tests Signed-off-by: Thane Thomson <connect@thanethomson.com> * Move CanonicalVoteExtension to canonical types proto defs Signed-off-by: Thane Thomson <connect@thanethomson.com> * Regenerate protos including latest PBTS synchrony params update Signed-off-by: Thane Thomson <connect@thanethomson.com> * Inject vote extensions into proposal Signed-off-by: Thane Thomson <connect@thanethomson.com> * Thread vote extensions through code and fix tests Signed-off-by: Thane Thomson <connect@thanethomson.com> * Remove extraneous empty value initialization Signed-off-by: Thane Thomson <connect@thanethomson.com> * Fix lint Signed-off-by: Thane Thomson <connect@thanethomson.com> * Fix missing VerifyVoteExtension request data Signed-off-by: Thane Thomson <connect@thanethomson.com> * Explicitly ensure length > 0 to sign vote extension Signed-off-by: Thane Thomson <connect@thanethomson.com> * Explicitly ensure length > 0 to sign vote extension Signed-off-by: Thane Thomson <connect@thanethomson.com> * Remove extraneous comment Signed-off-by: Thane Thomson <connect@thanethomson.com> * Update privval/file.go Co-authored-by: M. J. Fromberger <fromberger@interchain.io> * Update types/vote_test.go Co-authored-by: M. J. Fromberger <fromberger@interchain.io> * Format Signed-off-by: Thane Thomson <connect@thanethomson.com> * Fix ABCI proto generation scripts for Linux Signed-off-by: Thane Thomson <connect@thanethomson.com> * Sync intermediate and goal protos Signed-off-by: Thane Thomson <connect@thanethomson.com> * Update internal/consensus/common_test.go Co-authored-by: Sergio Mena <sergio@informal.systems> * Use dummy value with clearer meaning Signed-off-by: Thane Thomson <connect@thanethomson.com> * Rewrite loop for clarity Signed-off-by: Thane Thomson <connect@thanethomson.com> * Panic on ABCI++ method call failure Signed-off-by: Thane Thomson <connect@thanethomson.com> * Add strong correctness guarantees when constructing extended commit info for ABCI++ Signed-off-by: Thane Thomson <connect@thanethomson.com> * Add strong guarantee in extendedCommitInfo that the number of votes corresponds Signed-off-by: Thane Thomson <connect@thanethomson.com> * Make extendedCommitInfo function more robust At first extendedCommitInfo expected votes to be in the same order as their corresponding validators in the supplied CommitInfo struct, but this proved to be rather difficult since when a validator set's loaded from state it's first sorted by voting power and then by address. Instead of sorting the votes in the same way, this approach simply maps votes to their corresponding validator's address prior to constructing the extended commit info. This way it's easy to look up the corresponding vote and we don't need to care about vote order. Signed-off-by: Thane Thomson <connect@thanethomson.com> * Remove extraneous validator address assignment Signed-off-by: Thane Thomson <connect@thanethomson.com> * Sign over canonical vote extension Signed-off-by: Thane Thomson <connect@thanethomson.com> * Validate vote extension signature against canonical vote extension Signed-off-by: Thane Thomson <connect@thanethomson.com> * Update privval tests for more meaningful dummy value Signed-off-by: Thane Thomson <connect@thanethomson.com> * Add vote extension capability to E2E test app Signed-off-by: Thane Thomson <connect@thanethomson.com> * Disable lint for weak RNG usage for test app Signed-off-by: Thane Thomson <connect@thanethomson.com> * Use parseVoteExtension instead of custom parsing in PrepareProposal Signed-off-by: Thane Thomson <connect@thanethomson.com> * Only include extension if we have received txs It's unclear at this point why this is necessary to ensure that the application's local app_hash matches that committed in the previous block. Signed-off-by: Thane Thomson <connect@thanethomson.com> * Require app_hash from app to match that from last block Signed-off-by: Thane Thomson <connect@thanethomson.com> * Add contrived (possibly flaky) test to check that vote extensions code works Signed-off-by: Thane Thomson <connect@thanethomson.com> * Remove workaround for problem now solved by #8229 Signed-off-by: Thane Thomson <connect@thanethomson.com> * add tests for vote extension cases * Fix spelling mistake to appease linter Signed-off-by: Thane Thomson <connect@thanethomson.com> * Collapse redundant if statement Signed-off-by: Thane Thomson <connect@thanethomson.com> * Formatting Signed-off-by: Thane Thomson <connect@thanethomson.com> * Always expect an extension signature, regardless of whether an extension is present Signed-off-by: Thane Thomson <connect@thanethomson.com> * Votes constructed from commits cannot include extensions or signatures Signed-off-by: Thane Thomson <connect@thanethomson.com> * Pass through vote extension in test helpers Signed-off-by: Thane Thomson <connect@thanethomson.com> * Temporarily disable vote extension signature requirement Signed-off-by: Thane Thomson <connect@thanethomson.com> * Expand on vote equality test errors for clarity Signed-off-by: Thane Thomson <connect@thanethomson.com> * Expand on vote matching error messages in testing Signed-off-by: Thane Thomson <connect@thanethomson.com> * Allow for selective subscription by vote type This is an attempt to fix the intermittently failing `TestPrepareProposalReceivesVoteExtensions` test in the internal consensus package. Occasionally we get prevote messages via the subscription channel, and we're not interested in those. This change allows us to specify what types of votes we're interested in (i.e. precommits) and discard the rest. Signed-off-by: Thane Thomson <connect@thanethomson.com> * Read lock consensus state mutex in test helper to avoid data race Signed-off-by: Thane Thomson <connect@thanethomson.com> * Revert BlockIDFlag parameter in node test Signed-off-by: Thane Thomson <connect@thanethomson.com> * Perform additional check in ProcessProposal for special txs generated by vote extensions Signed-off-by: Thane Thomson <connect@thanethomson.com> * e2e: check that our added tx does not cause all txs to exceed req.MaxTxBytes Signed-off-by: Thane Thomson <connect@thanethomson.com> * Only set vote extension signatures when signing is successful Signed-off-by: Thane Thomson <connect@thanethomson.com> * Remove channel capacity constraint in test helper to avoid missing messages Signed-off-by: Thane Thomson <connect@thanethomson.com> * Add TODO to always require extension signatures in vote validation Signed-off-by: Thane Thomson <connect@thanethomson.com> * e2e: reject vote extensions if the request height does not match what we expect Signed-off-by: Thane Thomson <connect@thanethomson.com> * types: remove extraneous call to voteWithoutExtension in test Signed-off-by: Thane Thomson <connect@thanethomson.com> * Remove unnecessary address parameter from CanonicalVoteExtension Signed-off-by: Thane Thomson <connect@thanethomson.com> * privval: change test vote type to precommit since we use an extension Signed-off-by: Thane Thomson <connect@thanethomson.com> * privval: update signing logic to cater for vote extensions Signed-off-by: Thane Thomson <connect@thanethomson.com> * proto: update field descriptions for vote message Signed-off-by: Thane Thomson <connect@thanethomson.com> * proto: update field description for vote extension sig in vote message Signed-off-by: Thane Thomson <connect@thanethomson.com> * proto/types: use fixed-length 64-bit integers for rounds in CanonicalVoteExtension Signed-off-by: Thane Thomson <connect@thanethomson.com> * consensus: fix flaky TestPrepareProposalReceivesVoteExtensions Signed-off-by: Thane Thomson <connect@thanethomson.com> * consensus: remove previously added test helper functionality Signed-off-by: Thane Thomson <connect@thanethomson.com> * e2e: add error logs when we get an unexpected height in ExtendVote or VerifyVoteExtension requests Signed-off-by: Thane Thomson <connect@thanethomson.com> * node_test: get validator addresses from privvals Signed-off-by: Thane Thomson <connect@thanethomson.com> * privval/file_test: optimize filepv creation in tests Signed-off-by: Thane Thomson <connect@thanethomson.com> * privval: add test to check that vote extensions are always signed Signed-off-by: Thane Thomson <connect@thanethomson.com> * Add a script to check documentation for ToC entries. (#8356) This script verifies that each document in the docs and architecture directory has a corresponding table-of-contents entry in its README file. It can be run manually from the command line. - Hook up this script to run in CI (optional workflow). - Update ADR ToC to include missing entries this script found. * build(deps): Bump async from 2.6.3 to 2.6.4 in /docs (#8357) Bumps [async](https://github.com/caolan/async) from 2.6.3 to 2.6.4. - [Release notes](https://github.com/caolan/async/releases) - [Changelog](https://github.com/caolan/async/blob/v2.6.4/CHANGELOG.md) - [Commits](https://github.com/caolan/async/compare/v2.6.3...v2.6.4) --- updated-dependencies: - dependency-name: async dependency-type: indirect ... Signed-off-by: dependabot[bot] <support@github.com> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> * privval/file_test: reset vote ext sig before signing Signed-off-by: Thane Thomson <connect@thanethomson.com> Co-authored-by: M. J. Fromberger <fromberger@interchain.io> Co-authored-by: Sergio Mena <sergio@informal.systems> Co-authored-by: William Banfield <wbanfield@gmail.com> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
2026-01-08 22:23:11 +00:00 · 2022-04-19 08:05:17 -04:00
parent 9ea0e5efa7
commit a1104b98d2
35 changed files with 1813 additions and 1334 deletions
--- a/test/e2e/app/app.go
+++ b/test/e2e/app/app.go
@@ -3,20 +3,29 @@ package app
 import (
 	"bytes"
 	"encoding/base64"
+	"encoding/binary"
 	"errors"
 	"fmt"
+	"math/rand"
 	"path/filepath"
 	"sort"
 	"strconv"
+	"strings"
 	"sync"

 	"github.com/tendermint/tendermint/abci/example/code"
 	abci "github.com/tendermint/tendermint/abci/types"
+	"github.com/tendermint/tendermint/crypto"
 	"github.com/tendermint/tendermint/libs/log"
 	"github.com/tendermint/tendermint/proto/tendermint/types"
 	"github.com/tendermint/tendermint/version"
 )

+const (
+	voteExtensionKey    string = "extensionSum"
+	voteExtensionMaxVal int64  = 128
+)
+
 // Application is an ABCI application for use by end-to-end tests. It is a
 // simple key/value store for strings, storing data in memory and persisting
 // to disk as JSON, taking state sync snapshots if requested.
@@ -215,10 +224,10 @@ func (app *Application) Commit() abci.ResponseCommit {
 		if err != nil {
 			panic(err)
 		}
-		app.logger.Info("Created state sync snapshot", "height", snapshot.Height)
+		app.logger.Info("created state sync snapshot", "height", snapshot.Height)
 		err = app.snapshots.Prune(maxSnapshotCount)
 		if err != nil {
-			app.logger.Error("Failed to prune snapshots", "err", err)
+			app.logger.Error("failed to prune snapshots", "err", err)
 		}
 	}
 	retainHeight := int64(0)
@@ -304,7 +313,74 @@ func (app *Application) ApplySnapshotChunk(req abci.RequestApplySnapshotChunk) a
 	return abci.ResponseApplySnapshotChunk{Result: abci.ResponseApplySnapshotChunk_ACCEPT}
 }

+// PrepareProposal will take the given transactions and attempt to prepare a
+// proposal from them when it's our turn to do so. In the process, vote
+// extensions from the previous round of consensus, if present, will be used to
+// construct a special transaction whose value is the sum of all of the vote
+// extensions from the previous round.
+//
+// NB: Assumes that the supplied transactions do not exceed `req.MaxTxBytes`.
+// If adding a special vote extension-generated transaction would cause the
+// total number of transaction bytes to exceed `req.MaxTxBytes`, we will not
+// append our special vote extension transaction.
 func (app *Application) PrepareProposal(req abci.RequestPrepareProposal) abci.ResponsePrepareProposal {
+	var sum int64
+	var extCount int
+	for _, vote := range req.LocalLastCommit.Votes {
+		if !vote.SignedLastBlock || len(vote.VoteExtension) == 0 {
+			continue
+		}
+		extValue, err := parseVoteExtension(vote.VoteExtension)
+		// This should have been verified in VerifyVoteExtension
+		if err != nil {
+			panic(fmt.Errorf("failed to parse vote extension in PrepareProposal: %w", err))
+		}
+		valAddr := crypto.Address(vote.Validator.Address)
+		app.logger.Info("got vote extension value in PrepareProposal", "valAddr", valAddr, "value", extValue)
+		sum += extValue
+		extCount++
+	}
+	// We only generate our special transaction if we have vote extensions
+	if extCount > 0 {
+		var totalBytes int64
+		extTxPrefix := fmt.Sprintf("%s=", voteExtensionKey)
+		extTx := []byte(fmt.Sprintf("%s%d", extTxPrefix, sum))
+		app.logger.Info("preparing proposal with custom transaction from vote extensions", "tx", extTx)
+		// Our generated transaction takes precedence over any supplied
+		// transaction that attempts to modify the "extensionSum" value.
+		txRecords := make([]*abci.TxRecord, len(req.Txs)+1)
+		for i, tx := range req.Txs {
+			if strings.HasPrefix(string(tx), extTxPrefix) {
+				txRecords[i] = &abci.TxRecord{
+					Action: abci.TxRecord_REMOVED,
+					Tx:     tx,
+				}
+			} else {
+				txRecords[i] = &abci.TxRecord{
+					Action: abci.TxRecord_UNMODIFIED,
+					Tx:     tx,
+				}
+				totalBytes += int64(len(tx))
+			}
+		}
+		if totalBytes+int64(len(extTx)) < req.MaxTxBytes {
+			txRecords[len(req.Txs)] = &abci.TxRecord{
+				Action: abci.TxRecord_ADDED,
+				Tx:     extTx,
+			}
+		} else {
+			app.logger.Info(
+				"too many txs to include special vote extension-generated tx",
+				"totalBytes", totalBytes,
+				"MaxTxBytes", req.MaxTxBytes,
+				"extTx", extTx,
+				"extTxLen", len(extTx),
+			)
+		}
+		return abci.ResponsePrepareProposal{
+			TxRecords: txRecords,
+		}
+	}
 	// None of the transactions are modified by this application.
 	trs := make([]*abci.TxRecord, 0, len(req.Txs))
 	var totalBytes int64
@@ -325,14 +401,87 @@ func (app *Application) PrepareProposal(req abci.RequestPrepareProposal) abci.Re
 // It accepts any proposal that does not contain a malformed transaction.
 func (app *Application) ProcessProposal(req abci.RequestProcessProposal) abci.ResponseProcessProposal {
 	for _, tx := range req.Txs {
-		_, _, err := parseTx(tx)
+		k, v, err := parseTx(tx)
 		if err != nil {
+			app.logger.Error("malformed transaction in ProcessProposal", "tx", tx, "err", err)
 			return abci.ResponseProcessProposal{Status: abci.ResponseProcessProposal_REJECT}
 		}
+		// Additional check for vote extension-related txs
+		if k == voteExtensionKey {
+			_, err := strconv.Atoi(v)
+			if err != nil {
+				app.logger.Error("malformed vote extension transaction", k, v, "err", err)
+				return abci.ResponseProcessProposal{Status: abci.ResponseProcessProposal_REJECT}
+			}
+		}
 	}
 	return abci.ResponseProcessProposal{Status: abci.ResponseProcessProposal_ACCEPT}
 }

+// ExtendVote will produce vote extensions in the form of random numbers to
+// demonstrate vote extension nondeterminism.
+//
+// In the next block, if there are any vote extensions from the previous block,
+// a new transaction will be proposed that updates a special value in the
+// key/value store ("extensionSum") with the sum of all of the numbers collected
+// from the vote extensions.
+func (app *Application) ExtendVote(req abci.RequestExtendVote) abci.ResponseExtendVote {
+	// We ignore any requests for vote extensions that don't match our expected
+	// next height.
+	if req.Height != int64(app.state.Height)+1 {
+		app.logger.Error(
+			"got unexpected height in ExtendVote request",
+			"expectedHeight", app.state.Height+1,
+			"requestHeight", req.Height,
+		)
+		return abci.ResponseExtendVote{}
+	}
+	ext := make([]byte, binary.MaxVarintLen64)
+	// We don't care that these values are generated by a weak random number
+	// generator. It's just for test purposes.
+	// nolint:gosec // G404: Use of weak random number generator
+	num := rand.Int63n(voteExtensionMaxVal)
+	extLen := binary.PutVarint(ext, num)
+	app.logger.Info("generated vote extension", "num", num, "ext", fmt.Sprintf("%x", ext[:extLen]), "state.Height", app.state.Height)
+	return abci.ResponseExtendVote{
+		VoteExtension: ext[:extLen],
+	}
+}
+
+// VerifyVoteExtension simply validates vote extensions from other validators
+// without doing anything about them. In this case, it just makes sure that the
+// vote extension is a well-formed integer value.
+func (app *Application) VerifyVoteExtension(req abci.RequestVerifyVoteExtension) abci.ResponseVerifyVoteExtension {
+	// We allow vote extensions to be optional
+	if len(req.VoteExtension) == 0 {
+		return abci.ResponseVerifyVoteExtension{
+			Status: abci.ResponseVerifyVoteExtension_ACCEPT,
+		}
+	}
+	if req.Height != int64(app.state.Height)+1 {
+		app.logger.Error(
+			"got unexpected height in VerifyVoteExtension request",
+			"expectedHeight", app.state.Height,
+			"requestHeight", req.Height,
+		)
+		return abci.ResponseVerifyVoteExtension{
+			Status: abci.ResponseVerifyVoteExtension_REJECT,
+		}
+	}
+
+	num, err := parseVoteExtension(req.VoteExtension)
+	if err != nil {
+		app.logger.Error("failed to verify vote extension", "req", req, "err", err)
+		return abci.ResponseVerifyVoteExtension{
+			Status: abci.ResponseVerifyVoteExtension_REJECT,
+		}
+	}
+	app.logger.Info("verified vote extension value", "req", req, "num", num)
+	return abci.ResponseVerifyVoteExtension{
+		Status: abci.ResponseVerifyVoteExtension_ACCEPT,
+	}
+}
+
 func (app *Application) Rollback() error {
 	app.mu.Lock()
 	defer app.mu.Unlock()
@@ -378,3 +527,19 @@ func parseTx(tx []byte) (string, string, error) {
 	}
 	return string(parts[0]), string(parts[1]), nil
 }
+
+// parseVoteExtension attempts to parse the given extension data into a positive
+// integer value.
+func parseVoteExtension(ext []byte) (int64, error) {
+	num, errVal := binary.Varint(ext)
+	if errVal == 0 {
+		return 0, errors.New("vote extension is too small to parse")
+	}
+	if errVal < 0 {
+		return 0, errors.New("vote extension value is too large")
+	}
+	if num >= voteExtensionMaxVal {
+		return 0, fmt.Errorf("vote extension value must be smaller than %d (was %d)", voteExtensionMaxVal, num)
+	}
+	return num, nil
+}
--- a/test/e2e/tests/app_test.go
+++ b/test/e2e/tests/app_test.go
@@ -5,6 +5,7 @@ import (
 	"context"
 	"fmt"
 	"math/rand"
+	"strconv"
 	"testing"
 	"time"

@@ -45,9 +46,11 @@ func TestApp_Hash(t *testing.T) {
 	testNode(t, func(ctx context.Context, t *testing.T, node e2e.Node) {
 		client, err := node.Client()
 		require.NoError(t, err)
+
 		info, err := client.ABCIInfo(ctx)
 		require.NoError(t, err)
 		require.NotEmpty(t, info.Response.LastBlockAppHash, "expected app to return app hash")
+
 		// In next-block execution, the app hash is stored in the next block
 		blockHeight := info.Response.LastBlockHeight + 1

@@ -60,7 +63,6 @@ func TestApp_Hash(t *testing.T) {

 		block, err := client.Block(ctx, &blockHeight)
 		require.NoError(t, err)
-
 		require.Equal(t, blockHeight, block.Block.Height)
 		require.Equal(t,
 			fmt.Sprintf("%x", info.Response.LastBlockAppHash),
@@ -185,3 +187,18 @@ func TestApp_Tx(t *testing.T) {
 	}

 }
+
+func TestApp_VoteExtensions(t *testing.T) {
+	testNode(t, func(ctx context.Context, t *testing.T, node e2e.Node) {
+		client, err := node.Client()
+		require.NoError(t, err)
+
+		// This special value should have been created by way of vote extensions
+		resp, err := client.ABCIQuery(ctx, "", []byte("extensionSum"))
+		require.NoError(t, err)
+
+		extSum, err := strconv.Atoi(string(resp.Response.Value))
+		require.NoError(t, err)
+		require.GreaterOrEqual(t, extSum, 0)
+	})
+}