feat!: include state id in block signature (#478)

* chore: remove state id signature - main code building * chore(bytes): improve fixed-len serialization * test: update unit tests after state id refactor * fix(types): block ID of nil-block is nil * test(e2e) fix single tests after state id refactor * chore: state id refactor - self-review * refactor!: AppHash size MUST be 32 bytes * chore(types): stateID.AppVersion equals to Header.Version.App * doc: Document StateID encoding * chore(types): block ID IsNil should check StateID * refactor(types): change stateid.apphash to [32]byte * chore: move state ID to CanonicalVote and replace block id with hash of block id in CanonicalVote * fix(proto/types): correct handling of nil blockid in CanonicalVote * doc(consensus): update block/vote signing&verification * doc(core): revert fixed-length encoding info * chore: refactor after review feedback - WIP (tests don't build) * chore: minor fixes after state id refactor * test: update tests for state ID (except vectors / sizes) * chore: restore state ID hash in BlockID to not increase the block size * test: fix tests after state id refactor * chore: fix conflicts after merge 0.10-dev * doc(consensus): update vote/block signing with state id * refactor(types): remove unused QuorumSingsVerifier.shouldVerifyState * chore: fix typo * chore: make linter happy * refactor: move panic() from RoundState.BlockID() to Block.BlockID() * chore: minor code style changes * fix(e2e): ensure trusted light block already exists
dashpay · Nov 10, 2022 · 1e4faac · 1e4faac
1 parent dca7391
commit 1e4faac
Show file tree

Hide file tree

Showing 158 changed files with 3,261 additions and 3,425 deletions.
diff --git a/abci/client/doc.go b/abci/client/doc.go
@@ -1,9 +1,9 @@
 // Package abciclient provides an ABCI implementation in Go.
 //
 // There are 3 clients available:
-//		1. socket (unix or TCP)
-//		2. local (in memory)
-//		3. gRPC
+//  1. socket (unix or TCP)
+//  2. local (in memory)
+//  3. gRPC
 //
 // ## Socket client
 //
@@ -12,7 +12,7 @@
 //
 // ## Local client
 //
-// The global mutex is locked during each call
+// # The global mutex is locked during each call
 //
 // ## gRPC client
 //

diff --git a/abci/example/kvstore/kvstore.go b/abci/example/kvstore/kvstore.go
@@ -25,7 +25,7 @@ import (
 	"github.com/tendermint/tendermint/version"
 )
 
-const ProtocolVersion uint64 = 0x1
+const ProtocolVersion uint64 = 0x12345678
 
 //---------------------------------------------------
 

diff --git a/abci/server/server.go b/abci/server/server.go
@@ -2,9 +2,8 @@
 Package server is used to start a new ABCI server.
 
 It contains two server implementation:
- * gRPC server
- * socket server
-
+  - gRPC server
+  - socket server
 */
 package server
 

diff --git a/abci/types/application.go b/abci/types/application.go
@@ -6,9 +6,10 @@ import (
 	"github.com/tendermint/tendermint/crypto"
 )
 
-//go:generate ../../scripts/mockery_generate.sh Application
 // Application is an interface that enables any finite, deterministic state machine
 // to be driven by a blockchain-based replication engine via the ABCI.
+//
+//go:generate ../../scripts/mockery_generate.sh Application
 type Application interface {
 	// Info/Query Connection
 	Info(context.Context, *RequestInfo) (*ResponseInfo, error)    // Return application info

diff --git a/abci/types/types.go b/abci/types/types.go
@@ -276,7 +276,7 @@ func (m *ResponsePrepareProposal) Validate() error {
 }
 
 func isValidApphash(apphash tmbytes.HexBytes) bool {
-	return len(apphash) >= crypto.SmallAppHashSize && len(apphash) <= crypto.LargeAppHashSize
+	return len(apphash) == crypto.DefaultAppHashSize
 }
 
 func (r ResponseProcessProposal) Validate() error {

diff --git a/abci/types/types.pb.go b/abci/types/types.pb.go
diff --git a/config/config.go b/config/config.go
@@ -13,7 +13,6 @@ import (
 
 	"github.com/dashevo/dashd-go/btcjson"
 
-	"github.com/tendermint/tendermint/crypto"
 	"github.com/tendermint/tendermint/libs/log"
 	tmos "github.com/tendermint/tendermint/libs/os"
 	"github.com/tendermint/tendermint/types"
@@ -999,8 +998,6 @@ type ConsensusConfig struct {
 
 	QuorumType btcjson.LLMQType `mapstructure:"quorum-type"`
 
-	AppHashSize int `mapstructure:"app-hash-size"`
-
 	// TODO: The following fields are all temporary overrides that should exist only
 	// for the duration of the v0.36 release. The below fields should be completely
 	// removed in the v0.37 release of Tendermint.
@@ -1058,7 +1055,6 @@ func DefaultConsensusConfig() *ConsensusConfig {
 		PeerGossipSleepDuration:     100 * time.Millisecond,
 		PeerQueryMaj23SleepDuration: 2000 * time.Millisecond,
 		DoubleSignCheckHeight:       int64(0),
-		AppHashSize:                 crypto.DefaultAppHashSize,
 		QuorumType:                  btcjson.LLMQType_5_60,
 		ProposedBlockTimeWindow:     10 * time.Second,
 		DontAutoPropose:             false,
@@ -1071,7 +1067,6 @@ func TestConsensusConfig() *ConsensusConfig {
 	cfg.PeerGossipSleepDuration = 5 * time.Millisecond
 	cfg.PeerQueryMaj23SleepDuration = 250 * time.Millisecond
 	cfg.DoubleSignCheckHeight = int64(0)
-	cfg.AppHashSize = crypto.DefaultAppHashSize
 	cfg.QuorumType = btcjson.LLMQType_5_60
 	return cfg
 }
@@ -1165,12 +1160,14 @@ func (cfg *ConsensusConfig) DeprecatedFieldWarning() error {
 	return nil
 }
 
-//-----------------------------------------------------------------------------
+// -----------------------------------------------------------------------------
 // TxIndexConfig
 // Remember that Event has the following structure:
 // type: [
-//  key: value,
-//  ...
+//
+//	key: value,
+//	...
+//
 // ]
 //
 // CompositeKeys are constructed by `type.key`

diff --git a/config/toml.go b/config/toml.go
@@ -521,9 +521,6 @@ peer-query-maj23-sleep-duration = "{{ .Consensus.PeerQueryMaj23SleepDuration }}"
 # Signing parameters
 quorum-type = "{{ .Consensus.QuorumType }}"
 
-# State parameters
-app-hash-size = "{{ .Consensus.AppHashSize }}"
-
 #######################################################
 ###   Transaction Indexer Configuration Options     ###
 #######################################################

diff --git a/crypto/crypto.go b/crypto/crypto.go
@@ -20,9 +20,7 @@ const (
 	// AddressSize is the size of a pubkey address.
 	AddressSize        = 20
 	DefaultHashSize    = 32
-	LargeAppHashSize   = DefaultHashSize
-	SmallAppHashSize   = 20
-	DefaultAppHashSize = LargeAppHashSize
+	DefaultAppHashSize = DefaultHashSize
 	ProTxHashSize      = DefaultHashSize
 	QuorumHashSize     = DefaultHashSize
 )

diff --git a/crypto/merkle/doc.go b/crypto/merkle/doc.go
@@ -12,20 +12,19 @@ second pre-image attacks. Hence, use this library with caution.
 Otherwise you might run into similar issues as, e.g., in early Bitcoin:
 https://bitcointalk.org/?topic=102395
 
-                        *
-                       / \
-                     /     \
-                   /         \
-                 /             \
-                *               *
-               / \             / \
-              /   \           /   \
-             /     \         /     \
-            *       *       *       h6
-           / \     / \     / \
-          h0  h1  h2  h3  h4  h5
+	              *
+	             / \
+	           /     \
+	         /         \
+	       /             \
+	      *               *
+	     / \             / \
+	    /   \           /   \
+	   /     \         /     \
+	  *       *       *       h6
+	 / \     / \     / \
+	h0  h1  h2  h3  h4  h5
 
 TODO(ismail): add 2nd pre-image protection or clarify further on how we use this and why this secure.
-
 */
 package merkle
diff --git a/crypto/merkle/tree.go b/crypto/merkle/tree.go
@@ -53,10 +53,10 @@ func hashFromByteSlices(sha hash.Hash, items [][]byte) []byte {
 //
 // These preliminary results suggest:
 //
-// 1. The performance of the HashFromByteSlice is pretty good
-// 2. Go has low overhead for recursive functions
-// 3. The performance of the HashFromByteSlice routine is dominated
-//    by the actual hashing of data
+//  1. The performance of the HashFromByteSlice is pretty good
+//  2. Go has low overhead for recursive functions
+//  3. The performance of the HashFromByteSlice routine is dominated
+//     by the actual hashing of data
 //
 // Although this work is in no way exhaustive, point #3 suggests that
 // optimization of this routine would need to take an alternative

diff --git a/crypto/secp256k1/secp256k1.go b/crypto/secp256k1/secp256k1.go
@@ -20,7 +20,7 @@ import (
 	"golang.org/x/crypto/ripemd160" //nolint:staticcheck
 )
 
-//-------------------------------------
+// -------------------------------------
 const (
 	PrivKeyName = "tendermint/PrivKeySecp256k1"
 	PubKeyName  = "tendermint/PubKeySecp256k1"
@@ -198,8 +198,8 @@ func (pubKey PubKey) Equals(other crypto.PubKey) bool {
 
 // used to reject malleable signatures
 // see:
-//  - https://github.com/ethereum/go-ethereum/blob/f9401ae011ddf7f8d2d95020b7446c17f8d98dc1/crypto/signature_nocgo.go#L90-L93
-//  - https://github.com/ethereum/go-ethereum/blob/f9401ae011ddf7f8d2d95020b7446c17f8d98dc1/crypto/crypto.go#L39
+//   - https://github.com/ethereum/go-ethereum/blob/f9401ae011ddf7f8d2d95020b7446c17f8d98dc1/crypto/signature_nocgo.go#L90-L93
+//   - https://github.com/ethereum/go-ethereum/blob/f9401ae011ddf7f8d2d95020b7446c17f8d98dc1/crypto/crypto.go#L39
 var secp256k1halfN = new(big.Int).Rsh(secp256k1.S256().N, 1)
 
 // Sign creates an ECDSA signature on curve Secp256k1, using SHA256 on the msg.

diff --git a/dash/quorum/validator_conn_executor_test.go b/dash/quorum/validator_conn_executor_test.go
@@ -420,7 +420,7 @@ func TestFinalizeBlock(t *testing.T) {
 	require.NoError(t, err)
 
 	block := makeBlock(t, state, 1, new(types.Commit))
-	blockID, err := block.BlockID()
+	blockID := block.BlockID(nil)
 	require.NoError(t, err)
 	block.NextValidatorsHash = newVals.Hash()
 	const round = int32(0)

diff --git a/docs/tendermint-core/state-id.md b/docs/tendermint-core/state-id.md
diff --git a/internal/blocksync/pool.go b/internal/blocksync/pool.go
@@ -459,6 +459,7 @@ func (pool *BlockPool) sendError(err error, peerID types.NodeID) {
 }
 
 // for debugging purposes
+//
 //nolint:unused
 func (pool *BlockPool) debug() string {
 	pool.mtx.Lock()

diff --git a/internal/blocksync/reactor.go b/internal/blocksync/reactor.go
@@ -537,8 +537,11 @@ func (r *Reactor) poolRoutine(ctx context.Context, stateSynced bool, blockSyncCh
 
 			var (
 				firstPartSetHeader = firstParts.Header()
-				firstID            = types.BlockID{Hash: first.Hash(), PartSetHeader: firstPartSetHeader}
-				stateID            = first.StateID()
+				firstID            = types.BlockID{
+					Hash:          first.Hash(),
+					PartSetHeader: firstPartSetHeader,
+					StateID:       first.StateID().Hash(),
+				}
 			)
 
 			// Finally, verify the first block using the second's commit.
@@ -547,7 +550,7 @@ func (r *Reactor) poolRoutine(ctx context.Context, stateSynced bool, blockSyncCh
 			// first.Hash() doesn't verify the tx contents, so MakePartSet() is
 			// currently necessary.
 			// TODO(sergio): Should we also validate against the extended commit?
-			err = state.Validators.VerifyCommit(chainID, firstID, stateID, first.Height, second.LastCommit)
+			err = state.Validators.VerifyCommit(chainID, firstID, first.Height, second.LastCommit)
 
 			if err == nil {
 				// validate the block before we persist it

diff --git a/internal/blocksync/reactor_test.go b/internal/blocksync/reactor_test.go
@@ -14,7 +14,6 @@ import (
 	abciclient "github.com/tendermint/tendermint/abci/client"
 	abci "github.com/tendermint/tendermint/abci/types"
 	"github.com/tendermint/tendermint/config"
-	"github.com/tendermint/tendermint/crypto"
 	"github.com/tendermint/tendermint/internal/consensus"
 	"github.com/tendermint/tendermint/internal/eventbus"
 	mpmocks "github.com/tendermint/tendermint/internal/mempool/mocks"
@@ -59,7 +58,7 @@ func setup(
 	require.True(t, numNodes >= 1, "must specify at least one block height (nodes)")
 
 	rts := &reactorTestSuite{
-		logger:            log.NewNopLogger().With("module", "block_sync", "testCase", t.Name()),
+		logger:            log.NewTestingLogger(t).With("module", "block_sync", "testCase", t.Name()),
 		network:           p2ptest.MakeNetwork(ctx, t, p2ptest.NetworkOptions{NumNodes: numNodes}),
 		nodes:             make([]types.NodeID, 0, numNodes),
 		reactors:          make(map[types.NodeID]*Reactor, numNodes),
@@ -104,7 +103,7 @@ func makeReactor(
 	channelCreator p2p.ChannelCreator,
 	peerEvents p2p.PeerEventSubscriber) *Reactor {
 
-	logger := log.NewNopLogger()
+	logger := log.NewTestingLogger(t)
 
 	app := proxy.New(abciclient.NewLocalClient(logger, &abci.BaseApplication{}), logger, proxy.NopMetrics())
 	require.NoError(t, app.Start(ctx))
@@ -185,7 +184,7 @@ func (rts *reactorTestSuite) addNode(
 	peerEvents := func(ctx context.Context) *p2p.PeerUpdates { return rts.peerUpdates[nodeID] }
 	reactor := makeReactor(ctx, t, proTxHash, nodeID, genDoc, privVal, chCreator, peerEvents)
 
-	commit := types.NewCommit(0, 0, types.BlockID{}, types.StateID{}, nil)
+	commit := types.NewCommit(0, 0, types.BlockID{}, nil)
 
 	state, err := reactor.stateStore.Load()
 	require.NoError(t, err)
@@ -213,11 +212,8 @@ func makeNextBlock(ctx context.Context,
 	block.CoreChainLockedHeight = state.LastCoreChainLockedBlockHeight
 	partSet, err := block.MakePartSet(types.BlockPartSizeBytes)
 	require.NoError(t, err)
-	blockID := types.BlockID{Hash: block.Hash(), PartSetHeader: partSet.Header()}
-	stateID := types.StateID{
-		Height:  block.Header.Height,
-		AppHash: make([]byte, crypto.DefaultAppHashSize),
-	}
+	blockID := block.BlockID(partSet)
+	require.NoError(t, err)
 
 	// Simulate a commit for the current height
 	vote, err := factory.MakeVote(
@@ -230,18 +226,15 @@ func makeNextBlock(ctx context.Context,
 		0,
 		2,
 		blockID,
-		block.AppHash,
 	)
 	require.NoError(t, err)
 	seenCommit := types.NewCommit(
 		vote.Height,
 		vote.Round,
 		blockID,
-		stateID,
 		&types.CommitSigns{
 			QuorumSigns: types.QuorumSigns{
 				BlockSign:      vote.BlockSignature,
-				StateSign:      vote.StateSignature,
 				ExtensionSigns: types.MakeThresholdExtensionSigns(vote.VoteExtensions),
 			},
 			QuorumHash: state.Validators.QuorumHash,