node,blocksync,config: remove support for running nodes with blocksync disabled (#7159)

We stopped testing these configurations a while ago, and it doesn't
really make sense to allow nodes to run in this configuration. This
drops support for non-blocksync nodes and cleans up the
configuration/tests accordingly.

Closes: #6908

CHANGELOG_PENDING.md

@@ -12,21 +12,20 @@ Special thanks to external contributors on this release:
 
 - CLI/RPC/Config
 
-  - [rpc] Remove the deprecated gRPC interface to the RPC service (@creachadair).
+  - [rpc] Remove the deprecated gRPC interface to the RPC service. (@creachadair)
+  - [blocksync] \#7159 Remove support for disabling blocksync in any circumstance. (@tychoish)
 
 - Apps
+
   - [proto/tendermint] \#6976 Remove core protobuf files in favor of only housing them in the [tendermint/spec](https://github.com/tendermint/spec) repository.
 
 - P2P Protocol
 
-  - [p2p] \#7035 Remove legacy P2P routing implementation and
-    associated configuration options (@tychoish)
+  - [p2p] \#7035 Remove legacy P2P routing implementation and associated configuration options. (@tychoish)
 
 - Go API
 
-  - [blocksync] \#7046 Remove v2 implementation of the blocksync
-    service and recactor, which was disabled in the previous release
-    (@tychoish)
+  - [blocksync] \#7046 Remove v2 implementation of the blocksync service and recactor, which was disabled in the previous release. (@tychoish)
   - [p2p] \#7064 Remove WDRR queue implementation. (@tychoish)
 
 - Blockchain Protocol

cmd/tendermint/commands/run_node.go

@@ -3,8 +3,6 @@ package commands
 import (
 	"bytes"
 	"crypto/sha256"
-	"errors"
-	"flag"
 	"fmt"
 	"io"
 	"os"
@@ -35,22 +33,7 @@ func AddNodeFlags(cmd *cobra.Command) {
 		"socket address to listen on for connections from external priv-validator process")
 
 	// node flags
-	cmd.Flags().Bool("blocksync.enable", config.BlockSync.Enable, "enable fast blockchain syncing")
-
-	// TODO (https://github.com/tendermint/tendermint/issues/6908): remove this check after the v0.35 release cycle
-	// This check was added to give users an upgrade prompt to use the new flag for syncing.
-	//
-	// The pflag package does not have a native way to print a depcrecation warning
-	// and return an error. This logic was added to print a deprecation message to the user
-	// and then crash if the user attempts to use the old --fast-sync flag.
-	fs := flag.NewFlagSet("", flag.ExitOnError)
-	fs.Func("fast-sync", "deprecated",
-		func(string) error {
-			return errors.New("--fast-sync has been deprecated, please use --blocksync.enable")
-		})
-	cmd.Flags().AddGoFlagSet(fs)
-
-	cmd.Flags().MarkHidden("fast-sync") //nolint:errcheck
+
 	cmd.Flags().BytesHexVar(
 		&genesisHash,
 		"genesis-hash",

config/config.go

@@ -29,8 +29,6 @@ const (
 	ModeValidator = "validator"
 	ModeSeed      = "seed"
 
-	BlockSyncV0 = "v0"
-
 	MempoolV0 = "v0"
 	MempoolV1 = "v1"
 )
@@ -73,7 +71,6 @@ type Config struct {
 	P2P             *P2PConfig             `mapstructure:"p2p"`
 	Mempool         *MempoolConfig         `mapstructure:"mempool"`
 	StateSync       *StateSyncConfig       `mapstructure:"statesync"`
-	BlockSync       *BlockSyncConfig       `mapstructure:"blocksync"`
 	Consensus       *ConsensusConfig       `mapstructure:"consensus"`
 	TxIndex         *TxIndexConfig         `mapstructure:"tx-index"`
 	Instrumentation *InstrumentationConfig `mapstructure:"instrumentation"`
@@ -88,7 +85,6 @@ func DefaultConfig() *Config {
 		P2P:             DefaultP2PConfig(),
 		Mempool:         DefaultMempoolConfig(),
 		StateSync:       DefaultStateSyncConfig(),
-		BlockSync:       DefaultBlockSyncConfig(),
 		Consensus:       DefaultConsensusConfig(),
 		TxIndex:         DefaultTxIndexConfig(),
 		Instrumentation: DefaultInstrumentationConfig(),
@@ -111,7 +107,6 @@ func TestConfig() *Config {
 		P2P:             TestP2PConfig(),
 		Mempool:         TestMempoolConfig(),
 		StateSync:       TestStateSyncConfig(),
-		BlockSync:       TestBlockSyncConfig(),
 		Consensus:       TestConsensusConfig(),
 		TxIndex:         TestTxIndexConfig(),
 		Instrumentation: TestInstrumentationConfig(),
@@ -145,9 +140,6 @@ func (cfg *Config) ValidateBasic() error {
 	if err := cfg.StateSync.ValidateBasic(); err != nil {
 		return fmt.Errorf("error in [statesync] section: %w", err)
 	}
-	if err := cfg.BlockSync.ValidateBasic(); err != nil {
-		return fmt.Errorf("error in [blocksync] section: %w", err)
-	}
 	if err := cfg.Consensus.ValidateBasic(); err != nil {
 		return fmt.Errorf("error in [consensus] section: %w", err)
 	}
@@ -333,28 +325,6 @@ func (cfg BaseConfig) ValidateBasic() error {
 		return fmt.Errorf("unknown mode: %v", cfg.Mode)
 	}
 
-	// TODO (https://github.com/tendermint/tendermint/issues/6908) remove this check after the v0.35 release cycle.
-	// This check was added to give users an upgrade prompt to use the new
-	// configuration option in v0.35. In future release cycles they should no longer
-	// be using this configuration parameter so the check can be removed.
-	// The cfg.Other field can likely be removed at the same time if it is not referenced
-	// elsewhere as it was added to service this check.
-	if fs, ok := cfg.Other["fastsync"]; ok {
-		if _, ok := fs.(map[string]interface{}); ok {
-			return fmt.Errorf("a configuration section named 'fastsync' was found in the " +
-				"configuration file. The 'fastsync' section has been renamed to " +
-				"'blocksync', please update the 'fastsync' field in your configuration file to 'blocksync'")
-		}
-	}
-	if fs, ok := cfg.Other["fast-sync"]; ok {
-		if fs != "" {
-			return fmt.Errorf("a parameter named 'fast-sync' was found in the " +
-				"configuration file. The parameter to enable or disable quickly syncing with a blockchain" +
-				"has moved to the [blocksync] section of the configuration file as blocksync.enable. " +
-				"Please move the 'fast-sync' field in your configuration file to 'blocksync.enable'")
-		}
-	}
-
 	return nil
 }
 
@@ -943,31 +913,6 @@ func (cfg *StateSyncConfig) ValidateBasic() error {
 	return nil
 }
 
-//-----------------------------------------------------------------------------
-
-// BlockSyncConfig (formerly known as FastSync) defines the configuration for the Tendermint block sync service
-// If this node is many blocks behind the tip of the chain, BlockSync
-// allows them to catchup quickly by downloading blocks in parallel
-// and verifying their commits.
-type BlockSyncConfig struct {
-	Enable bool `mapstructure:"enable"`
-}
-
-// DefaultBlockSyncConfig returns a default configuration for the block sync service
-func DefaultBlockSyncConfig() *BlockSyncConfig {
-	return &BlockSyncConfig{
-		Enable: true,
-	}
-}
-
-// TestBlockSyncConfig returns a default configuration for the block sync.
-func TestBlockSyncConfig() *BlockSyncConfig {
-	return DefaultBlockSyncConfig()
-}
-
-// ValidateBasic performs basic validation.
-func (cfg *BlockSyncConfig) ValidateBasic() error { return nil }
-
 //-----------------------------------------------------------------------------
 // ConsensusConfig
 

config/config_test.go

@@ -104,11 +104,6 @@ func TestStateSyncConfigValidateBasic(t *testing.T) {
 	require.NoError(t, cfg.ValidateBasic())
 }
 
-func TestBlockSyncConfigValidateBasic(t *testing.T) {
-	cfg := TestBlockSyncConfig()
-	assert.NoError(t, cfg.ValidateBasic())
-}
-
 func TestConsensusConfig_ValidateBasic(t *testing.T) {
 	// nolint: lll
 	testcases := map[string]struct {

config/toml.go

@@ -415,16 +415,6 @@ chunk-request-timeout = "{{ .StateSync.ChunkRequestTimeout }}"
 # The number of concurrent chunk and block fetchers to run (default: 4).
 fetchers = "{{ .StateSync.Fetchers }}"
 
-#######################################################
-###       Block Sync Configuration Connections       ###
-#######################################################
-[blocksync]
-
-# If this node is many blocks behind the tip of the chain, BlockSync
-# allows them to catchup quickly by downloading blocks in parallel
-# and verifying their commits
-enable = {{ .BlockSync.Enable }}
-
 #######################################################
 ###         Consensus Configuration Options         ###
 #######################################################

config/toml_test.go

@@ -70,7 +70,6 @@ func checkConfig(t *testing.T, configFile string) {
 		"moniker",
 		"seeds",
 		"proxy-app",
-		"blocksync",
 		"create-empty-blocks",
 		"peer",
 		"timeout",

node/node.go

@@ -245,7 +245,7 @@ func makeNode(cfg *config.Config,
 
 	// Determine whether we should do block sync. This must happen after the handshake, since the
 	// app may modify the validator set, specifying ourself as the only validator.
-	blockSync := cfg.BlockSync.Enable && !onlyValidatorIsUs(state, pubKey)
+	blockSync := !onlyValidatorIsUs(state, pubKey)
 
 	logNodeStartupInfo(state, pubKey, logger, consensusLogger, cfg.Mode)
 
@@ -571,10 +571,8 @@ func (n *nodeImpl) OnStart() error {
 	}
 
 	if n.config.Mode != config.ModeSeed {
-		if n.config.BlockSync.Enable {
-			if err := n.bcReactor.Start(); err != nil {
-				return err
-			}
+		if err := n.bcReactor.Start(); err != nil {
+			return err
 		}
 
 		// Start the real consensus reactor separately since the switch uses the shim.
@@ -642,29 +640,32 @@ func (n *nodeImpl) OnStart() error {
 
 			n.consensusReactor.SetStateSyncingMetrics(0)
 
-			d := types.EventDataStateSyncStatus{Complete: true, Height: state.LastBlockHeight}
-			if err := n.eventBus.PublishEventStateSyncStatus(d); err != nil {
+			if err := n.eventBus.PublishEventStateSyncStatus(
+				types.EventDataStateSyncStatus{
+					Complete: true,
+					Height:   state.LastBlockHeight,
+				}); err != nil {
+
 				n.eventBus.Logger.Error("failed to emit the statesync start event", "err", err)
 			}
 
 			// TODO: Some form of orchestrator is needed here between the state
 			// advancing reactors to be able to control which one of the three
 			// is running
-			if n.config.BlockSync.Enable {
-				// FIXME Very ugly to have these metrics bleed through here.
-				n.consensusReactor.SetBlockSyncingMetrics(1)
-				if err := bcR.SwitchToBlockSync(state); err != nil {
-					n.Logger.Error("failed to switch to block sync", "err", err)
-					return
-				}
+			// FIXME Very ugly to have these metrics bleed through here.
+			n.consensusReactor.SetBlockSyncingMetrics(1)
+			if err := bcR.SwitchToBlockSync(state); err != nil {
+				n.Logger.Error("failed to switch to block sync", "err", err)
+				return
+			}
 
-				d := types.EventDataBlockSyncStatus{Complete: false, Height: state.LastBlockHeight}
-				if err := n.eventBus.PublishEventBlockSyncStatus(d); err != nil {
-					n.eventBus.Logger.Error("failed to emit the block sync starting event", "err", err)
-				}
+			if err := n.eventBus.PublishEventBlockSyncStatus(
+				types.EventDataBlockSyncStatus{
+					Complete: false,
+					Height:   state.LastBlockHeight,
+				}); err != nil {
 
-			} else {
-				n.consensusReactor.SwitchToConsensus(state, true)
+				n.eventBus.Logger.Error("failed to emit the block sync starting event", "err", err)
 			}
 		}()
 	}
@@ -697,11 +698,10 @@ func (n *nodeImpl) OnStop() {
 
 	if n.config.Mode != config.ModeSeed {
 		// now stop the reactors
-		if n.config.BlockSync.Enable {
-			// Stop the real blockchain reactor separately since the switch uses the shim.
-			if err := n.bcReactor.Stop(); err != nil {
-				n.Logger.Error("failed to stop the blockchain reactor", "err", err)
-			}
+
+		// Stop the real blockchain reactor separately since the switch uses the shim.
+		if err := n.bcReactor.Stop(); err != nil {
+			n.Logger.Error("failed to stop the blockchain reactor", "err", err)
 		}
 
 		// Stop the real consensus reactor separately since the switch uses the shim.

test/e2e/generator/generate.go

@@ -278,7 +278,6 @@ func generateNode(
 		Database:         nodeDatabases.Choose(r),
 		PrivvalProtocol:  nodePrivvalProtocols.Choose(r),
 		Mempool:          nodeMempools.Choose(r),
-		BlockSync:        "v0",
 		StateSync:        e2e.StateSyncDisabled,
 		PersistInterval:  ptrUint64(uint64(nodePersistIntervals.Choose(r).(int))),
 		SnapshotInterval: uint64(nodeSnapshotIntervals.Choose(r).(int)),

test/e2e/generator/generate_test.go

@@ -43,9 +43,6 @@ func TestGenerator(t *testing.T) {
 						t.Run("PrivvalProtocol", func(t *testing.T) {
 							require.NotZero(t, node.PrivvalProtocol)
 						})
-						t.Run("BlockSync", func(t *testing.T) {
-							require.NotZero(t, node.BlockSync)
-						})
 					}
 				})
 			}

test/e2e/pkg/manifest.go

@@ -104,10 +104,6 @@ type ManifestNode struct {
 	// runner will wait for the network to reach at least this block height.
 	StartAt int64 `toml:"start_at"`
 
-	// BlockSync specifies the block sync mode: "" (disable), "v0" or "v2".
-	// Defaults to disabled.
-	BlockSync string `toml:"block_sync"`
-
 	// Mempool specifies which version of mempool to use. Either "v0" or "v1"
 	Mempool string `toml:"mempool_version"`
 

test/e2e/pkg/testnet.go

@@ -84,7 +84,6 @@ type Node struct {
 	IP               net.IP
 	ProxyPort        uint32
 	StartAt          int64
-	BlockSync        string
 	Mempool          string
 	StateSync        string
 	Database         string
@@ -177,7 +176,6 @@ func LoadTestnet(file string) (*Testnet, error) {
 			ABCIProtocol:     Protocol(testnet.ABCIProtocol),
 			PrivvalProtocol:  ProtocolFile,
 			StartAt:          nodeManifest.StartAt,
-			BlockSync:        "v0",
 			Mempool:          nodeManifest.Mempool,
 			StateSync:        nodeManifest.StateSync,
 			PersistInterval:  1,
@@ -335,11 +333,6 @@ func (n Node) Validate(testnet Testnet) error {
 			}
 		}
 	}
-	switch n.BlockSync {
-	case "", "v0", "v2":
-	default:
-		return fmt.Errorf("invalid block sync setting %q", n.BlockSync)
-	}
 	switch n.StateSync {
 	case StateSyncDisabled, StateSyncP2P, StateSyncRPC:
 	default:

test/e2e/runner/setup.go

@@ -294,11 +294,6 @@ func MakeConfig(node *e2e.Node) (*config.Config, error) {
 		cfg.Mempool.Version = node.Mempool
 	}
 
-	cfg.BlockSync.Enable = true
-	if node.BlockSync == "" {
-		cfg.BlockSync.Enable = false
-	}
-
 	switch node.StateSync {
 	case e2e.StateSyncP2P:
 		cfg.StateSync.Enable = true