From cb39e2f91786efda5f60fdd67a72ba5ae8d07bb5 Mon Sep 17 00:00:00 2001 From: Sam Kleinman Date: Tue, 26 Oct 2021 16:35:14 +0200 Subject: [PATCH] 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 | 11 +++--- cmd/tendermint/commands/run_node.go | 19 +--------- config/config.go | 55 ----------------------------- config/config_test.go | 5 --- config/toml.go | 10 ------ config/toml_test.go | 1 - node/node.go | 50 +++++++++++++------------- test/e2e/generator/generate.go | 1 - test/e2e/generator/generate_test.go | 3 -- test/e2e/pkg/manifest.go | 4 --- test/e2e/pkg/testnet.go | 7 ---- test/e2e/runner/setup.go | 5 --- 12 files changed, 31 insertions(+), 140 deletions(-) diff --git a/CHANGELOG_PENDING.md b/CHANGELOG_PENDING.md index 33d2c89c2..98636a852 100644 --- a/CHANGELOG_PENDING.md +++ b/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 diff --git a/cmd/tendermint/commands/run_node.go b/cmd/tendermint/commands/run_node.go index a5fa72ed5..d5a3de04e 100644 --- a/cmd/tendermint/commands/run_node.go +++ b/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", diff --git a/config/config.go b/config/config.go index da1f56c34..c0443e021 100644 --- a/config/config.go +++ b/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 diff --git a/config/config_test.go b/config/config_test.go index 08a77d032..304eeb0ce 100644 --- a/config/config_test.go +++ b/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 { diff --git a/config/toml.go b/config/toml.go index 9c7d012f0..dc462243a 100644 --- a/config/toml.go +++ b/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 ### ####################################################### diff --git a/config/toml_test.go b/config/toml_test.go index ccf818d65..d46cf9ce7 100644 --- a/config/toml_test.go +++ b/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", diff --git a/node/node.go b/node/node.go index 727722279..d329e2494 100644 --- a/node/node.go +++ b/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. diff --git a/test/e2e/generator/generate.go b/test/e2e/generator/generate.go index 10a180f6a..4ee9849b2 100644 --- a/test/e2e/generator/generate.go +++ b/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)), diff --git a/test/e2e/generator/generate_test.go b/test/e2e/generator/generate_test.go index 389483027..3bda6d035 100644 --- a/test/e2e/generator/generate_test.go +++ b/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) - }) } }) } diff --git a/test/e2e/pkg/manifest.go b/test/e2e/pkg/manifest.go index 16b99cfda..b57b9ac4a 100644 --- a/test/e2e/pkg/manifest.go +++ b/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"` diff --git a/test/e2e/pkg/testnet.go b/test/e2e/pkg/testnet.go index d0770ce8d..3fd9e77a2 100644 --- a/test/e2e/pkg/testnet.go +++ b/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: diff --git a/test/e2e/runner/setup.go b/test/e2e/runner/setup.go index 9bf76c874..6199d804f 100644 --- a/test/e2e/runner/setup.go +++ b/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