From 14f41ac5e3d6f1dcf6dc062df903c6d7acf431b9 Mon Sep 17 00:00:00 2001 From: "M. J. Fromberger" Date: Thu, 7 Apr 2022 00:15:20 -0700 Subject: [PATCH 01/10] Fix more broken Markdown links. (#8271) --- CHANGELOG.md | 6 +++--- DOCKER/README.md | 2 +- UPGRADING.md | 2 +- crypto/README.md | 2 +- internal/p2p/README.md | 2 +- networks/local/README.md | 2 +- spec/p2p/messages/pex.md | 10 +++++----- test/e2e/README.md | 4 ++-- 8 files changed, 15 insertions(+), 15 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index f759c42c3..b4a022dcc 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2795,7 +2795,7 @@ are affected by a change. A few more breaking changes are in the works - each will come with a clear Architecture Decision Record (ADR) explaining the change. You can review ADRs -[here](https://github.com/tendermint/tendermint/tree/develop/docs/architecture) +[here](https://github.com/tendermint/tendermint/tree/master/docs/architecture) or in the [open Pull Requests](https://github.com/tendermint/tendermint/pulls). You can also check in on the [issues marked as breaking](https://github.com/tendermint/tendermint/issues?q=is%3Aopen+is%3Aissue+label%3Abreaking). @@ -3074,7 +3074,7 @@ BREAKING CHANGES: FEATURES - [cmd] Added metrics (served under `/metrics` using a Prometheus client; disabled by default). See the new `instrumentation` section in the config and - [metrics](https://tendermint.readthedocs.io/projects/tools/en/develop/metrics.html) + [metrics](https://github.com/tendermint/tendermint/blob/master/docs/nodes/metrics.md) guide. - [p2p] Add IPv6 support to peering. - [p2p] Add `external_address` to config to allow specifying the address for @@ -3188,7 +3188,7 @@ BREAKING: FEATURES -- [rpc] the RPC documentation is now published to https://tendermint.github.io/slate +- [rpc] the RPC documentation is now published to `https://tendermint.github.io/slate` - [p2p] AllowDuplicateIP config option to refuse connections from same IP. - true by default for now, false by default in next breaking release - [docs] Add docs for query, tx indexing, events, pubsub diff --git a/DOCKER/README.md b/DOCKER/README.md index 63c682811..4aa868e7a 100644 --- a/DOCKER/README.md +++ b/DOCKER/README.md @@ -8,7 +8,7 @@ Official releases can be found [here](https://github.com/tendermint/tendermint/r The Dockerfile for tendermint is not expected to change in the near future. The master file used for all builds can be found [here](https://raw.githubusercontent.com/tendermint/tendermint/master/DOCKER/Dockerfile). -Respective versioned files can be found (replace the Xs with the version number). +Respective versioned files can be found at `https://raw.githubusercontent.com/tendermint/tendermint/vX.XX.XX/DOCKER/Dockerfile` (replace the Xs with the version number). ## Quick reference diff --git a/UPGRADING.md b/UPGRADING.md index 6ae381b7b..931272cce 100644 --- a/UPGRADING.md +++ b/UPGRADING.md @@ -100,7 +100,7 @@ these parameters may do so by setting the `ConsensusParams.Timeout` field of the As a safety measure in case of unusual timing issues during the upgrade to v0.36, an operator may override the consensus timeout values for a single node. Note, however, that these overrides will be removed in Tendermint v0.37. See -[configuration](https://github.com/tendermint/tendermint/blob/wb/issue-8182/docs/nodes/configuration.md) +[configuration](https://github.com/tendermint/tendermint/blob/master/docs/nodes/configuration.md) for more information about these overrides. For more discussion of this, see [ADR 074](https://tinyurl.com/adr074), which diff --git a/crypto/README.md b/crypto/README.md index 20346d715..d60628d97 100644 --- a/crypto/README.md +++ b/crypto/README.md @@ -12,7 +12,7 @@ For any specific algorithm, use its specific module e.g. ## Binary encoding -For Binary encoding, please refer to the [Tendermint encoding specification](https://docs.tendermint.com/master/spec/blockchain/encoding.html). +For Binary encoding, please refer to the [Tendermint encoding specification](https://docs.tendermint.com/master/spec/core/encoding.html). ## JSON Encoding diff --git a/internal/p2p/README.md b/internal/p2p/README.md index 9ba7303fa..16ad1d5f6 100644 --- a/internal/p2p/README.md +++ b/internal/p2p/README.md @@ -7,5 +7,5 @@ Docs: - [Connection](https://docs.tendermint.com/master/spec/p2p/connection.html) for details on how connections and multiplexing work - [Peer](https://docs.tendermint.com/master/spec/p2p/node.html) for details on peer ID, handshakes, and peer exchange - [Node](https://docs.tendermint.com/master/spec/p2p/node.html) for details about different types of nodes and how they should work -- [Pex](https://docs.tendermint.com/master/spec/reactors/pex/pex.html) for details on peer discovery and exchange +- [Pex](https://docs.tendermint.com/master/spec/p2p/messages/pex.html) for details on peer discovery and exchange - [Config](https://docs.tendermint.com/master/spec/p2p/config.html) for details on some config option diff --git a/networks/local/README.md b/networks/local/README.md index dcb31ae71..10fc19932 100644 --- a/networks/local/README.md +++ b/networks/local/README.md @@ -1,3 +1,3 @@ # Local Cluster with Docker Compose -See the [docs](https://docs.tendermint.com/master/networks/docker-compose.html). +See the [docs](https://docs.tendermint.com/master/tools/docker-compose.html). diff --git a/spec/p2p/messages/pex.md b/spec/p2p/messages/pex.md index ea5986f0d..e02393d52 100644 --- a/spec/p2p/messages/pex.md +++ b/spec/p2p/messages/pex.md @@ -26,7 +26,7 @@ PexResponse is an list of net addresses provided to a peer to dial. | Name | Type | Description | Field Number | |-------|------------------------------------|------------------------------------------|--------------| -| addresses | repeated [PexAddress](#PexAddress) | List of peer addresses available to dial | 1 | +| addresses | repeated [PexAddress](#pexaddress) | List of peer addresses available to dial | 1 | ### PexAddress @@ -41,7 +41,7 @@ into a `NodeAddress`. See [ParseNodeAddress](https://github.com/tendermint/tende Message is a [`oneof` protobuf type](https://developers.google.com/protocol-buffers/docs/proto#oneof). The one of consists of two messages. -| Name | Type | Description | Field Number | -|--------------|---------------------------|------------------------------------------------------|--------------| -| pex_request | [PexRequest](#PexRequest) | Empty request asking for a list of addresses to dial | 3 | -| pex_response | [PexResponse](#PexResponse) | List of addresses to dial | 4 | +| Name | Type | Description | Field Number | +|--------------|-----------------------------|------------------------------------------------------|--------------| +| pex_request | [PexRequest](#pexrequest) | Empty request asking for a list of addresses to dial | 3 | +| pex_response | [PexResponse](#pexresponse) | List of addresses to dial | 4 | diff --git a/test/e2e/README.md b/test/e2e/README.md index 00bce5ad8..70510b6fa 100644 --- a/test/e2e/README.md +++ b/test/e2e/README.md @@ -11,7 +11,7 @@ This creates and runs a testnet named `ci` under `networks/ci/`. ## Conceptual Overview -End-to-end testnets are used to test Tendermint functionality as a user would use it, by spinning up a set of nodes with various configurations and making sure the nodes and network behave correctly. The background for the E2E test suite is outlined in [RFC-001](https://github.com/tendermint/tendermint/blob/master/docs/rfc/rfc-001-end-to-end-testing.md). +End-to-end testnets are used to test Tendermint functionality as a user would use it, by spinning up a set of nodes with various configurations and making sure the nodes and network behave correctly. The background for the E2E test suite is outlined in [RFC-001](https://github.com/tendermint/tendermint/blob/master/docs/architecture/adr-066-e2e-testing.md). The end-to-end tests can be thought of in this manner: @@ -180,4 +180,4 @@ tendermint start ./build/node ./node.socket.toml ``` -Check `node/config.go` to see how the settings of the test application can be tweaked. \ No newline at end of file +Check `node/config.go` to see how the settings of the test application can be tweaked. From 681cdf83470d85275f4249b0aeb351ebb3c6a07a Mon Sep 17 00:00:00 2001 From: Sam Kleinman Date: Thu, 7 Apr 2022 08:58:44 -0400 Subject: [PATCH 02/10] consensus: avoid panics during handshake (#8266) There's no case where we recieve an error during handshake and don't just return/continue, and it's at a point during startup where not much is going on in the process, so having some classes of errors return errors and some return panics is confusing and doesn't protect anything. --- internal/consensus/replay.go | 56 +++++++++++++++++++------------ internal/consensus/replay_test.go | 18 +++------- 2 files changed, 39 insertions(+), 35 deletions(-) diff --git a/internal/consensus/replay.go b/internal/consensus/replay.go index 177b9fbad..3b2dea930 100644 --- a/internal/consensus/replay.go +++ b/internal/consensus/replay.go @@ -220,7 +220,6 @@ func NewHandshaker( eventBus *eventbus.EventBus, genDoc *types.GenesisDoc, ) *Handshaker { - return &Handshaker{ stateStore: stateStore, initialState: state, @@ -228,7 +227,6 @@ func NewHandshaker( eventBus: eventBus, genDoc: genDoc, logger: logger, - nBlocks: 0, } } @@ -359,7 +357,9 @@ func (h *Handshaker) ReplayBlocks( // First handle edge cases and constraints on the storeBlockHeight and storeBlockBase. switch { case storeBlockHeight == 0: - assertAppHashEqualsOneFromState(appHash, state) + if err := checkAppHashEqualsOneFromState(appHash, state); err != nil { + return nil, err + } return appHash, nil case appBlockHeight == 0 && state.InitialHeight < storeBlockBase: @@ -376,11 +376,11 @@ func (h *Handshaker) ReplayBlocks( case storeBlockHeight < stateBlockHeight: // the state should never be ahead of the store (this is under tendermint's control) - panic(fmt.Sprintf("StateBlockHeight (%d) > StoreBlockHeight (%d)", stateBlockHeight, storeBlockHeight)) + return nil, fmt.Errorf("StateBlockHeight (%d) > StoreBlockHeight (%d)", stateBlockHeight, storeBlockHeight) case storeBlockHeight > stateBlockHeight+1: // store should be at most one ahead of the state (this is under tendermint's control) - panic(fmt.Sprintf("StoreBlockHeight (%d) > StateBlockHeight + 1 (%d)", storeBlockHeight, stateBlockHeight+1)) + return nil, fmt.Errorf("StoreBlockHeight (%d) > StateBlockHeight + 1 (%d)", storeBlockHeight, stateBlockHeight+1) } var err error @@ -395,7 +395,9 @@ func (h *Handshaker) ReplayBlocks( } else if appBlockHeight == storeBlockHeight { // We're good! - assertAppHashEqualsOneFromState(appHash, state) + if err := checkAppHashEqualsOneFromState(appHash, state); err != nil { + return nil, err + } return appHash, nil } @@ -415,7 +417,11 @@ func (h *Handshaker) ReplayBlocks( // but we'd have to allow the WAL to replay a block that wrote it's #ENDHEIGHT h.logger.Info("Replay last block using real app") state, err = h.replayBlock(ctx, state, storeBlockHeight, appClient) - return state.AppHash, err + if err != nil { + return nil, err + + } + return state.AppHash, nil case appBlockHeight == storeBlockHeight: // We ran Commit, but didn't save the state, so replayBlock with mock app. @@ -437,13 +443,13 @@ func (h *Handshaker) ReplayBlocks( return nil, err } - return state.AppHash, err + return state.AppHash, nil } } - panic(fmt.Sprintf("uncovered case! appHeight: %d, storeHeight: %d, stateHeight: %d", - appBlockHeight, storeBlockHeight, stateBlockHeight)) + return nil, fmt.Errorf("uncovered case! appHeight: %d, storeHeight: %d, stateHeight: %d", + appBlockHeight, storeBlockHeight, stateBlockHeight) } func (h *Handshaker) replayBlocks( @@ -452,7 +458,8 @@ func (h *Handshaker) replayBlocks( appClient abciclient.Client, appBlockHeight, storeBlockHeight int64, - mutateState bool) ([]byte, error) { + mutateState bool, +) ([]byte, error) { // App is further behind than it should be, so we need to replay blocks. // We replay all blocks from appBlockHeight+1. // @@ -478,7 +485,9 @@ func (h *Handshaker) replayBlocks( block := h.store.LoadBlock(i) // Extra check to ensure the app was not changed in a way it shouldn't have. if len(appHash) > 0 { - assertAppHashEqualsOneFromBlock(appHash, block) + if err := checkAppHashEqualsOneFromBlock(appHash, block); err != nil { + return nil, err + } } if i == finalBlock && !mutateState { @@ -510,7 +519,9 @@ func (h *Handshaker) replayBlocks( appHash = state.AppHash } - assertAppHashEqualsOneFromState(appHash, state) + if err := checkAppHashEqualsOneFromState(appHash, state); err != nil { + return nil, err + } return appHash, nil } @@ -539,24 +550,25 @@ func (h *Handshaker) replayBlock( return state, nil } -func assertAppHashEqualsOneFromBlock(appHash []byte, block *types.Block) { +func checkAppHashEqualsOneFromBlock(appHash []byte, block *types.Block) error { if !bytes.Equal(appHash, block.AppHash) { - panic(fmt.Sprintf(`block.AppHash does not match AppHash after replay. Got %X, expected %X. + return fmt.Errorf(`block.AppHash does not match AppHash after replay. Got '%X', expected '%X'. -Block: %v -`, - appHash, block.AppHash, block)) +Block: %v`, + appHash, block.AppHash, block) } + return nil } -func assertAppHashEqualsOneFromState(appHash []byte, state sm.State) { +func checkAppHashEqualsOneFromState(appHash []byte, state sm.State) error { if !bytes.Equal(appHash, state.AppHash) { - panic(fmt.Sprintf(`state.AppHash does not match AppHash after replay. Got -%X, expected %X. + return fmt.Errorf(`state.AppHash does not match AppHash after replay. Got '%X', expected '%X'. State: %v Did you reset Tendermint without resetting your application's data?`, - appHash, state.AppHash, state)) + appHash, state.AppHash, state) } + + return nil } diff --git a/internal/consensus/replay_test.go b/internal/consensus/replay_test.go index 468d912ac..6d9e82a05 100644 --- a/internal/consensus/replay_test.go +++ b/internal/consensus/replay_test.go @@ -944,7 +944,7 @@ func buildTMStateFromChain( return state } -func TestHandshakePanicsIfAppReturnsWrongAppHash(t *testing.T) { +func TestHandshakeErrorsIfAppReturnsWrongAppHash(t *testing.T) { // 1. Initialize tendermint and commit 3 blocks with the following app hashes: // - 0x01 // - 0x02 @@ -988,12 +988,8 @@ func TestHandshakePanicsIfAppReturnsWrongAppHash(t *testing.T) { require.NoError(t, err) t.Cleanup(func() { cancel(); proxyApp.Wait() }) - assert.Panics(t, func() { - h := NewHandshaker(logger, stateStore, state, store, eventBus, genDoc) - if err = h.Handshake(ctx, proxyApp); err != nil { - t.Log(err) - } - }) + h := NewHandshaker(logger, stateStore, state, store, eventBus, genDoc) + assert.Error(t, h.Handshake(ctx, proxyApp)) } // 3. Tendermint must panic if app returns wrong hash for the last block @@ -1008,12 +1004,8 @@ func TestHandshakePanicsIfAppReturnsWrongAppHash(t *testing.T) { require.NoError(t, err) t.Cleanup(func() { cancel(); proxyApp.Wait() }) - assert.Panics(t, func() { - h := NewHandshaker(logger, stateStore, state, store, eventBus, genDoc) - if err = h.Handshake(ctx, proxyApp); err != nil { - t.Log(err) - } - }) + h := NewHandshaker(logger, stateStore, state, store, eventBus, genDoc) + require.Error(t, h.Handshake(ctx, proxyApp)) } } From 6ed3f2d98d79ff78ae6b4d273df82205eb5d125b Mon Sep 17 00:00:00 2001 From: Sam Kleinman Date: Thu, 7 Apr 2022 11:21:10 -0400 Subject: [PATCH 03/10] node: move handshake out of constructor (#8264) --- internal/consensus/state.go | 20 +++++-- node/node.go | 115 ++++++++++++++++++------------------ node/node_test.go | 7 ++- node/setup.go | 8 +-- 4 files changed, 81 insertions(+), 69 deletions(-) diff --git a/internal/consensus/state.go b/internal/consensus/state.go index 670d0c42b..6ddc968ff 100644 --- a/internal/consensus/state.go +++ b/internal/consensus/state.go @@ -124,6 +124,7 @@ type State struct { stateStore sm.Store initialStatePopulated bool + skipBootstrapping bool // create and execute blocks blockExec *sm.BlockExecutor @@ -185,6 +186,12 @@ type State struct { // StateOption sets an optional parameter on the State. type StateOption func(*State) +// SkipStateStoreBootstrap is a state option forces the constructor to +// skip state bootstrapping during construction. +func SkipStateStoreBootstrap(sm *State) { + sm.skipBootstrapping = true +} + // NewState returns a new State. func NewState( ctx context.Context, @@ -223,16 +230,21 @@ func NewState( cs.doPrevote = cs.defaultDoPrevote cs.setProposal = cs.defaultSetProposal - if err := cs.updateStateFromStore(ctx); err != nil { - return nil, err - } - // NOTE: we do not call scheduleRound0 yet, we do that upon Start() cs.BaseService = *service.NewBaseService(logger, "State", cs) for _, option := range options { option(cs) } + // this is not ideal, but it lets the consensus tests start + // node-fragments gracefully while letting the nodes + // themselves avoid this. + if !cs.skipBootstrapping { + if err := cs.updateStateFromStore(ctx); err != nil { + return nil, err + } + } + return cs, nil } diff --git a/node/node.go b/node/node.go index 9b608d6f0..3ee75cfcf 100644 --- a/node/node.go +++ b/node/node.go @@ -61,17 +61,18 @@ type nodeImpl struct { // services eventSinks []indexer.EventSink + initialState sm.State stateStore sm.Store blockStore *store.BlockStore // store the blockchain to disk evPool *evidence.Pool stateSync bool // whether the node should state sync on startup stateSyncReactor *statesync.Reactor // for hosting and restoring state sync snapshots - - services []service.Service - rpcListeners []net.Listener // rpc servers - shutdownOps closer - rpcEnv *rpccore.Environment - prometheusSrv *http.Server + indexerService *indexer.Service + services []service.Service + rpcListeners []net.Listener // rpc servers + shutdownOps closer + rpcEnv *rpccore.Environment + prometheusSrv *http.Server } // newDefaultNode returns a Tendermint node with default settings for the @@ -157,20 +158,8 @@ func makeNode( nodeMetrics := defaultMetricsProvider(cfg.Instrumentation)(genDoc.ChainID) - // Create the proxyApp and establish connections to the ABCI app (consensus, mempool, query). proxyApp := proxy.New(client, logger.With("module", "proxy"), nodeMetrics.proxy) - if err := proxyApp.Start(ctx); err != nil { - return nil, fmt.Errorf("error starting proxy app connections: %w", err) - } - - // EventBus and IndexerService must be started before the handshake because - // we might need to index the txs of the replayed block as this might not have happened - // when the node stopped last time (i.e. the node stopped or crashed after it saved the block - // but before it indexed the txs) eventBus := eventbus.NewDefault(logger.With("module", "events")) - if err := eventBus.Start(ctx); err != nil { - return nil, combineCloseError(err, makeCloser(closers)) - } var eventLog *eventlog.Log if w := cfg.RPC.EventLogWindowSize; w > 0 { @@ -185,13 +174,11 @@ func makeNode( } } - indexerService, eventSinks, err := createAndStartIndexerService( - ctx, cfg, dbProvider, eventBus, - logger, genDoc.ChainID, nodeMetrics.indexer) + indexerService, eventSinks, err := createIndexerService( + cfg, dbProvider, eventBus, logger, genDoc.ChainID, nodeMetrics.indexer) if err != nil { return nil, combineCloseError(err, makeCloser(closers)) } - closers = append(closers, func() error { indexerService.Stop(); return nil }) privValidator, err := createPrivval(ctx, logger, cfg, genDoc, filePrivval) if err != nil { @@ -213,34 +200,6 @@ func makeNode( } } - // Create the handshaker, which calls RequestInfo, sets the AppVersion on the state, - // and replays any blocks as necessary to sync tendermint with the app. - if err := consensus.NewHandshaker( - logger.With("module", "handshaker"), - stateStore, state, blockStore, eventBus, genDoc, - ).Handshake(ctx, proxyApp); err != nil { - return nil, combineCloseError(err, makeCloser(closers)) - } - - // Reload the state. It will have the Version.Consensus.App set by the - // Handshake, and may have other modifications as well (ie. depending on - // what happened during block replay). - state, err = stateStore.Load() - if err != nil { - return nil, combineCloseError( - fmt.Errorf("cannot load state: %w", err), - makeCloser(closers)) - } - - logNodeStartupInfo(state, pubKey, logger, cfg.Mode) - - // TODO: Fetch and provide real options and do proper p2p bootstrapping. - // TODO: Use a persistent peer database. - nodeInfo, err := makeNodeInfo(cfg, nodeKey, eventSinks, genDoc, state.Version.Consensus) - if err != nil { - return nil, combineCloseError(err, makeCloser(closers)) - } - peerManager, peerCloser, err := createPeerManager(cfg, dbProvider, nodeKey.ID) closers = append(closers, peerCloser) if err != nil { @@ -257,15 +216,15 @@ func makeNode( privValidator: privValidator, peerManager: peerManager, - nodeInfo: nodeInfo, nodeKey: nodeKey, - eventSinks: eventSinks, + eventSinks: eventSinks, + indexerService: indexerService, + services: []service.Service{eventBus}, - services: []service.Service{eventBus}, - - stateStore: stateStore, - blockStore: blockStore, + initialState: state, + stateStore: stateStore, + blockStore: blockStore, shutdownOps: makeCloser(closers), @@ -408,6 +367,48 @@ func makeNode( // OnStart starts the Node. It implements service.Service. func (n *nodeImpl) OnStart(ctx context.Context) error { + if err := n.rpcEnv.ProxyApp.Start(ctx); err != nil { + return fmt.Errorf("error starting proxy app connections: %w", err) + } + + // EventBus and IndexerService must be started before the handshake because + // we might need to index the txs of the replayed block as this might not have happened + // when the node stopped last time (i.e. the node stopped or crashed after it saved the block + // but before it indexed the txs) + if err := n.rpcEnv.EventBus.Start(ctx); err != nil { + return err + } + + if err := n.indexerService.Start(ctx); err != nil { + return err + } + + // Create the handshaker, which calls RequestInfo, sets the AppVersion on the state, + // and replays any blocks as necessary to sync tendermint with the app. + if err := consensus.NewHandshaker(n.logger.With("module", "handshaker"), + n.stateStore, n.initialState, n.blockStore, n.rpcEnv.EventBus, n.genesisDoc, + ).Handshake(ctx, n.rpcEnv.ProxyApp); err != nil { + return err + } + + // Reload the state. It will have the Version.Consensus.App set by the + // Handshake, and may have other modifications as well (ie. depending on + // what happened during block replay). + state, err := n.stateStore.Load() + if err != nil { + return fmt.Errorf("cannot load state: %w", err) + } + + logNodeStartupInfo(state, n.rpcEnv.PubKey, n.logger, n.config.Mode) + + // TODO: Fetch and provide real options and do proper p2p bootstrapping. + // TODO: Use a persistent peer database. + n.nodeInfo, err = makeNodeInfo(n.config, n.nodeKey, n.eventSinks, n.genesisDoc, state.Version.Consensus) + if err != nil { + return err + } + // Start Internal Services + if n.config.RPC.PprofListenAddress != "" { rpcCtx, rpcCancel := context.WithCancel(ctx) srv := &http.Server{Addr: n.config.RPC.PprofListenAddress, Handler: nil} @@ -445,7 +446,7 @@ func (n *nodeImpl) OnStart(ctx context.Context) error { } } - state, err := n.stateStore.Load() + state, err = n.stateStore.Load() if err != nil { return err } diff --git a/node/node_test.go b/node/node_test.go index 1a1fa6f81..2736ca818 100644 --- a/node/node_test.go +++ b/node/node_test.go @@ -62,12 +62,13 @@ func TestNodeStartStop(t *testing.T) { require.NoError(t, n.Start(ctx)) // wait for the node to produce a block - tctx, cancel := context.WithTimeout(ctx, time.Second) + tctx, cancel := context.WithTimeout(ctx, 10*time.Second) defer cancel() blocksSub, err := n.EventBus().SubscribeWithArgs(tctx, pubsub.SubscribeArgs{ ClientID: "node_test", Query: types.EventQueryNewBlock, + Limit: 1000, }) require.NoError(t, err) _, err = blocksSub.Next(tctx) @@ -138,6 +139,8 @@ func TestNodeSetAppVersion(t *testing.T) { // create node n := getTestNode(ctx, t, cfg, logger) + require.NoError(t, n.Start(ctx)) + // default config uses the kvstore app appVersion := kvstore.ProtocolVersion @@ -624,7 +627,7 @@ func TestNodeSetEventSink(t *testing.T) { genDoc, err := types.GenesisDocFromFile(cfg.GenesisFile()) require.NoError(t, err) - indexService, eventSinks, err := createAndStartIndexerService(ctx, cfg, + indexService, eventSinks, err := createIndexerService(cfg, config.DefaultDBProvider, eventBus, logger, genDoc.ChainID, indexer.NopMetrics()) require.NoError(t, err) diff --git a/node/setup.go b/node/setup.go index e87fac79c..1057fb6fa 100644 --- a/node/setup.go +++ b/node/setup.go @@ -95,8 +95,7 @@ func initDBs( return blockStore, stateDB, makeCloser(closers), nil } -func createAndStartIndexerService( - ctx context.Context, +func createIndexerService( cfg *config.Config, dbProvider config.DBProvider, eventBus *eventbus.EventBus, @@ -116,10 +115,6 @@ func createAndStartIndexerService( Metrics: metrics, }) - if err := indexerService.Start(ctx); err != nil { - return nil, nil, err - } - return indexerService, eventSinks, nil } @@ -264,6 +259,7 @@ func createConsensusReactor( evidencePool, eventBus, consensus.StateMetrics(csMetrics), + consensus.SkipStateStoreBootstrap, ) if err != nil { return nil, nil, err From 9d20e0690041b693b183000d39a2c12801521a8c Mon Sep 17 00:00:00 2001 From: Sam Kleinman Date: Thu, 7 Apr 2022 16:23:36 -0400 Subject: [PATCH 04/10] statesync+blocksync: move event publications into the sync operations (#8274) --- internal/blocksync/reactor.go | 7 +++++++ internal/consensus/reactor.go | 24 ----------------------- internal/statesync/reactor.go | 35 ++++++++++++++++++++------------- node/node.go | 37 +---------------------------------- types/validator_set.go | 5 +---- 5 files changed, 30 insertions(+), 78 deletions(-) diff --git a/internal/blocksync/reactor.go b/internal/blocksync/reactor.go index 6533fa046..6f3743e62 100644 --- a/internal/blocksync/reactor.go +++ b/internal/blocksync/reactor.go @@ -359,6 +359,13 @@ func (r *Reactor) SwitchToBlockSync(ctx context.Context, state sm.State) error { go r.requestRoutine(ctx, bsCh) go r.poolRoutine(ctx, true, bsCh) + if err := r.PublishStatus(ctx, types.EventDataBlockSyncStatus{ + Complete: false, + Height: state.LastBlockHeight, + }); err != nil { + return err + } + return nil } diff --git a/internal/consensus/reactor.go b/internal/consensus/reactor.go index 1c6c99cb9..c73f998d8 100644 --- a/internal/consensus/reactor.go +++ b/internal/consensus/reactor.go @@ -752,9 +752,6 @@ func (r *Reactor) gossipVotesForHeight( func (r *Reactor) gossipVotesRoutine(ctx context.Context, ps *PeerState, voteCh *p2p.Channel) { logger := r.logger.With("peer", ps.peerID) - // XXX: simple hack to throttle logs upon sleep - logThrottle := 0 - timer := time.NewTimer(0) defer timer.Stop() @@ -772,13 +769,6 @@ func (r *Reactor) gossipVotesRoutine(ctx context.Context, ps *PeerState, voteCh rs := r.getRoundState() prs := ps.GetRoundState() - switch logThrottle { - case 1: // first sleep - logThrottle = 2 - case 2: // no more sleep - logThrottle = 0 - } - // if height matches, then send LastCommit, Prevotes, and Precommits if rs.Height == prs.Height { if ok, err := r.gossipVotesForHeight(ctx, rs, prs, ps, voteCh); err != nil { @@ -813,20 +803,6 @@ func (r *Reactor) gossipVotesRoutine(ctx context.Context, ps *PeerState, voteCh } } - if logThrottle == 0 { - // we sent nothing -- sleep - logThrottle = 1 - logger.Debug( - "no votes to send; sleeping", - "rs.Height", rs.Height, - "prs.Height", prs.Height, - "localPV", rs.Votes.Prevotes(rs.Round).BitArray(), "peerPV", prs.Prevotes, - "localPC", rs.Votes.Precommits(rs.Round).BitArray(), "peerPC", prs.Precommits, - ) - } else if logThrottle == 2 { - logThrottle = 1 - } - timer.Reset(r.state.config.PeerGossipSleepDuration) select { case <-ctx.Done(): diff --git a/internal/statesync/reactor.go b/internal/statesync/reactor.go index 15abf3ef0..5ca1d0798 100644 --- a/internal/statesync/reactor.go +++ b/internal/statesync/reactor.go @@ -310,20 +310,21 @@ func (r *Reactor) OnStop() { r.dispatcher.Close() } -func (r *Reactor) PublishStatus(ctx context.Context, event types.EventDataStateSyncStatus) error { - if r.eventBus == nil { - return errors.New("event system is not configured") - } - - return r.eventBus.PublishEventStateSyncStatus(ctx, event) -} - // Sync runs a state sync, fetching snapshots and providing chunks to the // application. At the close of the operation, Sync will bootstrap the state // store and persist the commit at that height so that either consensus or // blocksync can commence. It will then proceed to backfill the necessary amount // of historical blocks before participating in consensus func (r *Reactor) Sync(ctx context.Context) (sm.State, error) { + if r.eventBus != nil { + if err := r.eventBus.PublishEventStateSyncStatus(ctx, types.EventDataStateSyncStatus{ + Complete: false, + Height: r.initialHeight, + }); err != nil { + return sm.State{}, err + } + } + // We need at least two peers (for cross-referencing of light blocks) before we can // begin state sync if err := r.waitForEnoughPeers(ctx, 2); err != nil { @@ -357,21 +358,27 @@ func (r *Reactor) Sync(ctx context.Context) (sm.State, error) { return sm.State{}, err } - err = r.stateStore.Bootstrap(state) - if err != nil { + if err := r.stateStore.Bootstrap(state); err != nil { return sm.State{}, fmt.Errorf("failed to bootstrap node with new state: %w", err) } - err = r.blockStore.SaveSeenCommit(state.LastBlockHeight, commit) - if err != nil { + if err := r.blockStore.SaveSeenCommit(state.LastBlockHeight, commit); err != nil { return sm.State{}, fmt.Errorf("failed to store last seen commit: %w", err) } - err = r.Backfill(ctx, state) - if err != nil { + if err := r.Backfill(ctx, state); err != nil { r.logger.Error("backfill failed. Proceeding optimistically...", "err", err) } + if r.eventBus != nil { + if err := r.eventBus.PublishEventStateSyncStatus(ctx, types.EventDataStateSyncStatus{ + Complete: true, + Height: state.LastBlockHeight, + }); err != nil { + return sm.State{}, err + } + } + return state, nil } diff --git a/node/node.go b/node/node.go index 3ee75cfcf..37b66c697 100644 --- a/node/node.go +++ b/node/node.go @@ -489,23 +489,6 @@ func (n *nodeImpl) OnStart(ctx context.Context) error { // TODO: We shouldn't run state sync if we already have state that has a // LastBlockHeight that is not InitialHeight if n.stateSync { - bcR := n.rpcEnv.BlockSyncReactor - - // we need to get the genesis state to get parameters such as - state, err := sm.MakeGenesisState(n.genesisDoc) - if err != nil { - return fmt.Errorf("unable to derive state: %w", err) - } - - // TODO: we may want to move these events within the respective - // reactors. - // At the beginning of the statesync start, we use the initialHeight as the event height - // because of the statesync doesn't have the concreate state height before fetched the snapshot. - d := types.EventDataStateSyncStatus{Complete: false, Height: state.InitialHeight} - if err := n.stateSyncReactor.PublishStatus(ctx, d); err != nil { - n.logger.Error("failed to emit the statesync start event", "err", err) - } - // RUN STATE SYNC NOW: // // TODO: Eventually this should run as part of some @@ -521,33 +504,15 @@ func (n *nodeImpl) OnStart(ctx context.Context) error { n.rpcEnv.ConsensusReactor.SetStateSyncingMetrics(0) - if err := n.stateSyncReactor.PublishStatus(ctx, - types.EventDataStateSyncStatus{ - Complete: true, - Height: ssState.LastBlockHeight, - }); err != nil { - n.logger.Error("failed to emit the statesync start event", "err", err) - return 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 // FIXME Very ugly to have these metrics bleed through here. n.rpcEnv.ConsensusReactor.SetBlockSyncingMetrics(1) - if err := bcR.SwitchToBlockSync(ctx, ssState); err != nil { + if err := n.rpcEnv.BlockSyncReactor.SwitchToBlockSync(ctx, ssState); err != nil { n.logger.Error("failed to switch to block sync", "err", err) return err } - - if err := bcR.PublishStatus(ctx, - types.EventDataBlockSyncStatus{ - Complete: false, - Height: ssState.LastBlockHeight, - }); err != nil { - n.logger.Error("failed to emit the block sync starting event", "err", err) - return err - } } return nil diff --git a/types/validator_set.go b/types/validator_set.go index 1f773d1b5..d825ff590 100644 --- a/types/validator_set.go +++ b/types/validator_set.go @@ -68,7 +68,7 @@ type ValidatorSet struct { // MaxVotesCount - commits by a validator set larger than this will fail // validation. func NewValidatorSet(valz []*Validator) *ValidatorSet { - vals := &ValidatorSet{} + vals := &ValidatorSet{Validators: []*Validator{}} err := vals.updateWithChangeSet(valz, false) if err != nil { panic(fmt.Errorf("cannot create validator set: %w", err)) @@ -235,9 +235,6 @@ func (vals *ValidatorSet) shiftByAvgProposerPriority() { // Makes a copy of the validator list. func validatorListCopy(valsList []*Validator) []*Validator { - if valsList == nil { - return nil - } valsCopy := make([]*Validator, len(valsList)) for i, val := range valsList { valsCopy[i] = val.Copy() From f3858e52def8051575db7a0fea907e8f10bfb506 Mon Sep 17 00:00:00 2001 From: "M. J. Fromberger" Date: Thu, 7 Apr 2022 15:09:09 -0700 Subject: [PATCH 05/10] scmigrate: ensure target key is correctly renamed (#8276) Prior to v0.35, the keys for seen-commit records included the applicable height. In v0.35 and beyond, we only keep the record for the latest height, and its key does not include the height. Update the seen-commit migration to ensure that the record we retain after migration is correctly renamed to omit the height from its key. Update the test cases to check for this condition after migrating. --- CHANGELOG_PENDING.md | 1 + go.mod | 1 + go.sum | 2 + scripts/keymigrate/migrate.go | 82 +++++++----------------------- scripts/keymigrate/migrate_test.go | 17 ++----- scripts/scmigrate/migrate.go | 51 ++++++++++++++++--- scripts/scmigrate/migrate_test.go | 26 +++++----- 7 files changed, 81 insertions(+), 99 deletions(-) diff --git a/CHANGELOG_PENDING.md b/CHANGELOG_PENDING.md index cc71a5a5e..f0409a545 100644 --- a/CHANGELOG_PENDING.md +++ b/CHANGELOG_PENDING.md @@ -81,3 +81,4 @@ Special thanks to external contributors on this release: - [light] \#7640 Light Client: fix absence proof verification (@ashcherbakov) - [light] \#7641 Light Client: fix querying against the latest height (@ashcherbakov) - [cli] [#7837](https://github.com/tendermint/tendermint/pull/7837) fix app hash in state rollback. (@yihuang) +- [cli] \#8276 scmigrate: ensure target key is correctly renamed. (@creachadair) diff --git a/go.mod b/go.mod index 3b0004ba0..49db1ba5e 100644 --- a/go.mod +++ b/go.mod @@ -72,6 +72,7 @@ require ( github.com/charithe/durationcheck v0.0.9 // indirect github.com/chavacava/garif v0.0.0-20210405164556-e8a0a408d6af // indirect github.com/containerd/continuity v0.2.1 // indirect + github.com/creachadair/taskgroup v0.3.2 // indirect github.com/daixiang0/gci v0.3.3 // indirect github.com/davecgh/go-spew v1.1.1 // indirect github.com/denis-tingaikin/go-header v0.4.3 // indirect diff --git a/go.sum b/go.sum index 7374c90d0..c0388b7af 100644 --- a/go.sum +++ b/go.sum @@ -223,6 +223,8 @@ github.com/cpuguy83/go-md2man/v2 v2.0.0/go.mod h1:maD7wRr/U5Z6m/iR4s+kqSMx2CaBsr github.com/cpuguy83/go-md2man/v2 v2.0.1/go.mod h1:tgQtvFlXSQOSOSIRvRPT7W67SCa46tRHOmNcaadrF8o= github.com/creachadair/atomicfile v0.2.4 h1:GRjpQLmz/78I4+nBQpGMFrRa9yrL157AUTrA6hnF0YU= github.com/creachadair/atomicfile v0.2.4/go.mod h1:BRq8Une6ckFneYXZQ+kO7p1ZZP3I2fzVzf28JxrIkBc= +github.com/creachadair/taskgroup v0.3.2 h1:zlfutDS+5XG40AOxcHDSThxKzns8Tnr9jnr6VqkYlkM= +github.com/creachadair/taskgroup v0.3.2/go.mod h1:wieWwecHVzsidg2CsUnFinW1faVN4+kq+TDlRJQ0Wbk= github.com/creack/pty v1.1.7/go.mod h1:lj5s0c3V2DBrqTV7llrYr5NG6My20zk30Fl46Y7DoTY= github.com/creack/pty v1.1.9/go.mod h1:oKZEueFk5CKHvIhNR5MUki03XCEU+Q6VDXinZuGJ33E= github.com/cyphar/filepath-securejoin v0.2.2/go.mod h1:FpkQEhXnPnOthhzymB7CGsFk2G9VLXONKD9G7QGMM+4= diff --git a/scripts/keymigrate/migrate.go b/scripts/keymigrate/migrate.go index 2061223ad..b53c5e98f 100644 --- a/scripts/keymigrate/migrate.go +++ b/scripts/keymigrate/migrate.go @@ -15,8 +15,8 @@ import ( "math/rand" "runtime" "strconv" - "sync" + "github.com/creachadair/taskgroup" "github.com/google/orderedcode" dbm "github.com/tendermint/tm-db" ) @@ -27,7 +27,7 @@ type ( ) func getAllLegacyKeys(db dbm.DB) ([]keyID, error) { - out := []keyID{} + var out []keyID iter, err := db.Iterator(nil, nil) if err != nil { @@ -43,11 +43,8 @@ func getAllLegacyKeys(db dbm.DB) ([]keyID, error) { continue } - // there's inconsistency around tm-db's handling of - // key copies. - nk := make([]byte, len(k)) - copy(nk, k) - out = append(out, nk) + // Make an explicit copy, since not all tm-db backends do. + out = append(out, []byte(string(k))) } if err = iter.Error(); err != nil { @@ -61,17 +58,6 @@ func getAllLegacyKeys(db dbm.DB) ([]keyID, error) { return out, nil } -func makeKeyChan(keys []keyID) <-chan keyID { - out := make(chan keyID, len(keys)) - defer close(out) - - for _, key := range keys { - out <- key - } - - return out -} - func keyIsLegacy(key keyID) bool { for _, prefix := range []keyID{ // core "store" @@ -111,7 +97,7 @@ func keyIsHash(key keyID) bool { return len(key) == 32 && !bytes.Contains(key, []byte("/")) } -func migarateKey(key keyID) (keyID, error) { +func migrateKey(key keyID) (keyID, error) { switch { case bytes.HasPrefix(key, keyID("H:")): val, err := strconv.Atoi(string(key[2:])) @@ -349,53 +335,23 @@ func Migrate(ctx context.Context, db dbm.DB) error { return err } - numWorkers := runtime.NumCPU() - wg := &sync.WaitGroup{} + var errs []string + g, start := taskgroup.New(func(err error) error { + errs = append(errs, err.Error()) + return err + }).Limit(runtime.NumCPU()) - errs := make(chan error, numWorkers) - - keyCh := makeKeyChan(keys) - - // run migrations. - for i := 0; i < numWorkers; i++ { - wg.Add(1) - go func() { - defer wg.Done() - for key := range keyCh { - err := replaceKey(db, key, migarateKey) - if err != nil { - errs <- err - } - - if ctx.Err() != nil { - return - } + for _, key := range keys { + key := key + start(func() error { + if err := ctx.Err(); err != nil { + return err } - }() + return replaceKey(db, key, migrateKey) + }) } - - // collect and process the errors. - errStrs := []string{} - signal := make(chan struct{}) - go func() { - defer close(signal) - for err := range errs { - if err == nil { - continue - } - errStrs = append(errStrs, err.Error()) - } - }() - - // Wait for everything to be done. - wg.Wait() - close(errs) - <-signal - - // check the error results - if len(errs) != 0 { - return fmt.Errorf("encountered errors during migration: %v", errStrs) + if g.Wait() != nil { + return fmt.Errorf("encountered errors during migration: %q", errs) } - return nil } diff --git a/scripts/keymigrate/migrate_test.go b/scripts/keymigrate/migrate_test.go index 21e9592fb..8f6f30808 100644 --- a/scripts/keymigrate/migrate_test.go +++ b/scripts/keymigrate/migrate_test.go @@ -117,7 +117,7 @@ func TestMigration(t *testing.T) { }) t.Run("Conversion", func(t *testing.T) { for kind, le := range legacyPrefixes { - nk, err := migarateKey(le) + nk, err := migrateKey(le) require.NoError(t, err, kind) require.False(t, keyIsLegacy(nk), kind) } @@ -159,7 +159,7 @@ func TestMigration(t *testing.T) { "UserKey3": []byte("foo/bar/baz/1.2/4"), } for kind, key := range table { - out, err := migarateKey(key) + out, err := migrateKey(key) require.Error(t, err, kind) require.Nil(t, out, kind) } @@ -177,7 +177,7 @@ func TestMigration(t *testing.T) { return nil, errors.New("hi") })) }) - t.Run("KeyDisapears", func(t *testing.T) { + t.Run("KeyDisappears", func(t *testing.T) { db := dbm.NewMemDB() key := keyID("hi") require.NoError(t, db.Set(key, []byte("world"))) @@ -215,17 +215,6 @@ func TestMigration(t *testing.T) { require.False(t, keyIsLegacy(key)) } }) - t.Run("ChannelConversion", func(t *testing.T) { - ch := makeKeyChan([]keyID{ - makeKey(t, "abc", int64(2), int64(42)), - makeKey(t, int64(42)), - }) - count := 0 - for range ch { - count++ - } - require.Equal(t, 2, count) - }) t.Run("Migrate", func(t *testing.T) { _, db := getLegacyDatabase(t) diff --git a/scripts/scmigrate/migrate.go b/scripts/scmigrate/migrate.go index e6eee3d95..ba3de2698 100644 --- a/scripts/scmigrate/migrate.go +++ b/scripts/scmigrate/migrate.go @@ -73,8 +73,6 @@ func sortMigrations(scData []toMigrate) { }) } -func getMigrationsToDelete(in []toMigrate) []toMigrate { return in[1:] } - func getAllSeenCommits(ctx context.Context, db dbm.DB) ([]toMigrate, error) { scKeyPrefix := makeKeyFromPrefix(prefixSeenCommit) iter, err := db.Iterator( @@ -117,6 +115,34 @@ func getAllSeenCommits(ctx context.Context, db dbm.DB) ([]toMigrate, error) { return scData, nil } +func renameRecord(ctx context.Context, db dbm.DB, keep toMigrate) error { + wantKey := makeKeyFromPrefix(prefixSeenCommit) + if bytes.Equal(keep.key, wantKey) { + return nil // we already did this conversion + } + + // This record's key has already been converted to the "new" format, we just + // now need to trim off the tail. + val, err := db.Get(keep.key) + if err != nil { + return err + } + + batch := db.NewBatch() + if err := batch.Delete(keep.key); err != nil { + return err + } + if err := batch.Set(wantKey, val); err != nil { + return err + } + werr := batch.Write() + cerr := batch.Close() + if werr != nil { + return werr + } + return cerr +} + func deleteRecords(ctx context.Context, db dbm.DB, scData []toMigrate) error { // delete all the remaining stale values in a single batch batch := db.NewBatch() @@ -141,20 +167,29 @@ func Migrate(ctx context.Context, db dbm.DB) error { scData, err := getAllSeenCommits(ctx, db) if err != nil { return fmt.Errorf("sourcing tasks to migrate: %w", err) + } else if len(scData) == 0 { + return nil // nothing to do } - // sort earliest->latest commits. + // Sort commits in decreasing order of height. sortMigrations(scData) - // trim the one we want to save: - scData = getMigrationsToDelete(scData) + // Keep and rename the newest seen commit, delete the rest. + // In TM < v0.35 we kept a last-seen commit for each height; in v0.35 we + // retain only the latest. + keep, remove := scData[0], scData[1:] - if len(scData) <= 1 { + if err := renameRecord(ctx, db, keep); err != nil { + return fmt.Errorf("renaming seen commit record: %w", err) + } + + if len(remove) == 0 { return nil } - // write the migration (remove ) - if err := deleteRecords(ctx, db, scData); err != nil { + // Remove any older seen commits. Prior to v0.35, we kept these records for + // all heights, but v0.35 keeps only the latest. + if err := deleteRecords(ctx, db, remove); err != nil { return fmt.Errorf("writing data: %w", err) } diff --git a/scripts/scmigrate/migrate_test.go b/scripts/scmigrate/migrate_test.go index abe12584d..900a15f85 100644 --- a/scripts/scmigrate/migrate_test.go +++ b/scripts/scmigrate/migrate_test.go @@ -1,6 +1,7 @@ package scmigrate import ( + "bytes" "context" "math/rand" "testing" @@ -104,15 +105,6 @@ func TestMigrations(t *testing.T) { assertWellOrderedMigrations(t, testData) }) }) - t.Run("GetMigrationsToDelete", func(t *testing.T) { - for i := 1; i < 100; i++ { - data := appendRandomMigrations([]toMigrate{}, i) - toMigrate := getMigrationsToDelete(data) - if len(data) != len(toMigrate)+1 { - t.Fatalf("migration prep did not save one document [original=%d migrations=%d]", len(data), len(toMigrate)) - } - } - }) t.Run("InvalidMigrations", func(t *testing.T) { if _, err := makeToMigrate(nil); err == nil { t.Fatal("should error for nil migrations") @@ -156,18 +148,24 @@ func TestMigrations(t *testing.T) { // safe to rerun t.Run(test, func(t *testing.T) { if err := Migrate(ctx, db); err != nil { - t.Fatal(err) + t.Fatalf("Migration failed: %v", err) } post, err := getAllSeenCommits(ctx, db) if err != nil { - t.Fatal(err) + t.Fatalf("Fetching seen commits: %v", err) } + if len(post) != 1 { - t.Fatal("migration was not successful") + t.Fatalf("Wrong number of commits: got %d, wanted 1", len(post)) } - if post[0].commit.Height != latestHeight { - t.Fatal("migration did not save correct document") + + wantKey := makeKeyFromPrefix(prefixSeenCommit) + if !bytes.Equal(post[0].key, wantKey) { + t.Errorf("Seen commit key: got %x, want %x", post[0].key, wantKey) + } + if got := post[0].commit.Height; got != latestHeight { + t.Fatalf("Wrong commit height after migration: got %d, wanted %d", got, latestHeight) } }) } From 53b7dbe285fdbbd142d4d15337a5e026280653a0 Mon Sep 17 00:00:00 2001 From: "M. J. Fromberger" Date: Thu, 7 Apr 2022 15:30:48 -0700 Subject: [PATCH 06/10] Forward-port changelog for v0.34.19 to master. (#8279) --- CHANGELOG.md | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index b4a022dcc..07d049cf2 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -230,6 +230,12 @@ Special thanks to external contributors on this release: @JayT106, - [cmd/tendermint/commands] [\#6623](https://github.com/tendermint/tendermint/pull/6623) replace `$HOME/.some/test/dir` with `t.TempDir` (@tanyabouman) - [statesync] \6807 Implement P2P state provider as an alternative to RPC (@cmwaters) +## v0.34.19 + +### BUG FIXES + +- [cli] [\#8270](https://github.com/tendermint/tendermint/issues/8270) fix reset commands (@alexanderbez). + ## v0.34.18 ### BREAKING CHANGES From 5df277cacaf27088863f69ab656ae968f1a53089 Mon Sep 17 00:00:00 2001 From: "dependabot[bot]" <49699333+dependabot[bot]@users.noreply.github.com> Date: Fri, 8 Apr 2022 13:19:28 +0000 Subject: [PATCH 07/10] build(deps): Bump github.com/lib/pq from 1.10.4 to 1.10.5 (#8283) Bumps [github.com/lib/pq](https://github.com/lib/pq) from 1.10.4 to 1.10.5.
Commits

[![Dependabot compatibility score](https://dependabot-badges.githubapp.com/badges/compatibility_score?dependency-name=github.com/lib/pq&package-manager=go_modules&previous-version=1.10.4&new-version=1.10.5)](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 | 4 ++-- go.sum | 3 ++- 2 files changed, 4 insertions(+), 3 deletions(-) diff --git a/go.mod b/go.mod index 49db1ba5e..b25b8ef39 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.4 + github.com/lib/pq v1.10.5 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 @@ -39,6 +39,7 @@ require ( require ( github.com/creachadair/atomicfile v0.2.4 + github.com/creachadair/taskgroup v0.3.2 github.com/golangci/golangci-lint v1.45.2 github.com/google/go-cmp v0.5.7 github.com/vektra/mockery/v2 v2.10.4 @@ -72,7 +73,6 @@ require ( github.com/charithe/durationcheck v0.0.9 // indirect github.com/chavacava/garif v0.0.0-20210405164556-e8a0a408d6af // indirect github.com/containerd/continuity v0.2.1 // indirect - github.com/creachadair/taskgroup v0.3.2 // indirect github.com/daixiang0/gci v0.3.3 // indirect github.com/davecgh/go-spew v1.1.1 // indirect github.com/denis-tingaikin/go-header v0.4.3 // indirect diff --git a/go.sum b/go.sum index c0388b7af..4fc7fab1e 100644 --- a/go.sum +++ b/go.sum @@ -658,8 +658,9 @@ github.com/letsencrypt/pkcs11key/v4 v4.0.0/go.mod h1:EFUvBDay26dErnNb70Nd0/VW3tJ 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 h1:SO9z7FRPzA03QhHKJrH5BXA6HU1rS4V2nIVrrNC1iYk= 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/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/logrusorgru/aurora v0.0.0-20181002194514-a7b3b318ed4e/go.mod h1:7rIyQOR62GCctdiQpZ/zOJlFyk6y+94wXzv6RNZgaR4= From 90b951af7210a0275f79b3075e420c9a3f465a75 Mon Sep 17 00:00:00 2001 From: Sam Kleinman Date: Fri, 8 Apr 2022 11:00:58 -0400 Subject: [PATCH 08/10] node+statesync: normalize initialization (#8275) --- internal/statesync/reactor.go | 54 ++++++++++++------ internal/statesync/reactor_test.go | 5 +- node/node.go | 88 +++++++++++------------------- 3 files changed, 74 insertions(+), 73 deletions(-) diff --git a/internal/statesync/reactor.go b/internal/statesync/reactor.go index 5ca1d0798..ea2cac4f4 100644 --- a/internal/statesync/reactor.go +++ b/internal/statesync/reactor.go @@ -143,6 +143,12 @@ type Reactor struct { peerEvents p2p.PeerEventSubscriber chCreator p2p.ChannelCreator sendBlockError func(context.Context, p2p.PeerError) error + postSyncHook func(context.Context, sm.State) error + + // when true, the reactor will, during startup perform a + // statesync for this node, and otherwise just provide + // snapshots to other nodes. + needsStateSync bool // Dispatcher is used to multiplex light block requests and responses over multiple // peers used by the p2p state provider and in reverse sync. @@ -171,7 +177,6 @@ type Reactor struct { // and querying, references to p2p Channels and a channel to listen for peer // updates on. Note, the reactor will close all p2p Channels when stopping. func NewReactor( - ctx context.Context, chainID string, initialHeight int64, cfg config.StateSyncConfig, @@ -184,23 +189,26 @@ func NewReactor( tempDir string, ssMetrics *Metrics, eventBus *eventbus.EventBus, + postSyncHook func(context.Context, sm.State) error, + needsStateSync bool, ) *Reactor { - r := &Reactor{ - logger: logger, - chainID: chainID, - initialHeight: initialHeight, - cfg: cfg, - conn: conn, - chCreator: channelCreator, - peerEvents: peerEvents, - tempDir: tempDir, - stateStore: stateStore, - blockStore: blockStore, - peers: newPeerList(), - providers: make(map[types.NodeID]*BlockProvider), - metrics: ssMetrics, - eventBus: eventBus, + logger: logger, + chainID: chainID, + initialHeight: initialHeight, + cfg: cfg, + conn: conn, + chCreator: channelCreator, + peerEvents: peerEvents, + tempDir: tempDir, + stateStore: stateStore, + blockStore: blockStore, + peers: newPeerList(), + providers: make(map[types.NodeID]*BlockProvider), + metrics: ssMetrics, + eventBus: eventBus, + postSyncHook: postSyncHook, + needsStateSync: needsStateSync, } r.BaseService = *service.NewBaseService(logger, "StateSync", r) @@ -300,6 +308,14 @@ func (r *Reactor) OnStart(ctx context.Context) error { go r.processChannels(ctx, snapshotCh, chunkCh, blockCh, paramsCh) go r.processPeerUpdates(ctx, r.peerEvents(ctx)) + if r.needsStateSync { + r.logger.Info("starting state sync") + if _, err := r.Sync(ctx); err != nil { + r.logger.Error("state sync failed; shutting down this node", "err", err) + return err + } + } + return nil } @@ -379,6 +395,12 @@ func (r *Reactor) Sync(ctx context.Context) (sm.State, error) { } } + if r.postSyncHook != nil { + if err := r.postSyncHook(ctx, state); err != nil { + return sm.State{}, err + } + } + return state, nil } diff --git a/internal/statesync/reactor_test.go b/internal/statesync/reactor_test.go index cef0735f2..86c84a9ed 100644 --- a/internal/statesync/reactor_test.go +++ b/internal/statesync/reactor_test.go @@ -155,7 +155,6 @@ func setup( logger := log.NewNopLogger() rts.reactor = NewReactor( - ctx, factory.DefaultTestChainID, 1, *cfg, @@ -167,7 +166,9 @@ func setup( rts.blockStore, "", m, - nil, // eventbus can be nil + nil, // eventbus can be nil + nil, // post-sync-hook + false, // run Sync during Start() ) rts.syncer = &syncer{ diff --git a/node/node.go b/node/node.go index 37b66c697..8a4da29f2 100644 --- a/node/node.go +++ b/node/node.go @@ -60,19 +60,17 @@ type nodeImpl struct { nodeKey types.NodeKey // our node privkey // services - eventSinks []indexer.EventSink - initialState sm.State - stateStore sm.Store - blockStore *store.BlockStore // store the blockchain to disk - evPool *evidence.Pool - stateSync bool // whether the node should state sync on startup - stateSyncReactor *statesync.Reactor // for hosting and restoring state sync snapshots - indexerService *indexer.Service - services []service.Service - rpcListeners []net.Listener // rpc servers - shutdownOps closer - rpcEnv *rpccore.Environment - prometheusSrv *http.Server + eventSinks []indexer.EventSink + initialState sm.State + stateStore sm.Store + blockStore *store.BlockStore // store the blockchain to disk + evPool *evidence.Pool + indexerService *indexer.Service + services []service.Service + rpcListeners []net.Listener // rpc servers + shutdownOps closer + rpcEnv *rpccore.Environment + prometheusSrv *http.Server } // newDefaultNode returns a Tendermint node with default settings for the @@ -331,13 +329,15 @@ func makeNode( nodeMetrics.consensus.BlockSyncing.Set(1) } + if cfg.P2P.PexReactor { + node.services = append(node.services, pex.NewReactor(logger, peerManager, node.router.OpenChannel, peerManager.Subscribe)) + } + // Set up state sync reactor, and schedule a sync if requested. // FIXME The way we do phased startups (e.g. replay -> block sync -> consensus) is very messy, // we should clean this whole thing up. See: // https://github.com/tendermint/tendermint/issues/4644 - node.stateSync = stateSync - node.stateSyncReactor = statesync.NewReactor( - ctx, + node.services = append(node.services, statesync.NewReactor( genDoc.ChainID, genDoc.InitialHeight, *cfg.StateSync, @@ -350,11 +350,24 @@ func makeNode( cfg.StateSync.TempDir, nodeMetrics.statesync, eventBus, - ) + // the post-sync operation + func(ctx context.Context, state sm.State) error { + csReactor.SetStateSyncingMetrics(0) - if cfg.P2P.PexReactor { - node.services = append(node.services, pex.NewReactor(logger, peerManager, node.router.OpenChannel, peerManager.Subscribe)) - } + // 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 + // FIXME Very ugly to have these metrics bleed through here. + csReactor.SetBlockSyncingMetrics(1) + if err := bcReactor.SwitchToBlockSync(ctx, state); err != nil { + logger.Error("failed to switch to block sync", "err", err) + return err + } + + return nil + }, + stateSync, + )) if cfg.Mode == config.ModeValidator { node.rpcEnv.PubKey = pubKey @@ -481,40 +494,6 @@ func (n *nodeImpl) OnStart(ctx context.Context) error { } } - if err := n.stateSyncReactor.Start(ctx); err != nil { - return err - } - - // Run state sync - // TODO: We shouldn't run state sync if we already have state that has a - // LastBlockHeight that is not InitialHeight - if n.stateSync { - // RUN STATE SYNC NOW: - // - // TODO: Eventually this should run as part of some - // separate orchestrator - n.logger.Info("starting state sync") - ssState, err := n.stateSyncReactor.Sync(ctx) - if err != nil { - n.logger.Error("state sync failed; shutting down this node", "err", err) - // stop the node - n.Stop() - return err - } - - n.rpcEnv.ConsensusReactor.SetStateSyncingMetrics(0) - - // 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 - // FIXME Very ugly to have these metrics bleed through here. - n.rpcEnv.ConsensusReactor.SetBlockSyncingMetrics(1) - if err := n.rpcEnv.BlockSyncReactor.SwitchToBlockSync(ctx, ssState); err != nil { - n.logger.Error("failed to switch to block sync", "err", err) - return err - } - } - return nil } @@ -531,7 +510,6 @@ func (n *nodeImpl) OnStop() { reactor.Wait() } - n.stateSyncReactor.Wait() n.router.Wait() n.rpcEnv.IsListening = false From 3e3a9348180b9e3a8a48d887e3e5864624ed347b Mon Sep 17 00:00:00 2001 From: Sam Kleinman Date: Fri, 8 Apr 2022 14:04:43 -0400 Subject: [PATCH 09/10] rpc: add more nil checks in the status end point (#8287) --- internal/rpc/core/status.go | 22 ++++++++++++++++++---- 1 file changed, 18 insertions(+), 4 deletions(-) diff --git a/internal/rpc/core/status.go b/internal/rpc/core/status.go index 46b8a6fcd..870c13424 100644 --- a/internal/rpc/core/status.go +++ b/internal/rpc/core/status.go @@ -77,14 +77,25 @@ func (env *Environment) Status(ctx context.Context) (*coretypes.ResultStatus, er EarliestAppHash: earliestAppHash, EarliestBlockHeight: earliestBlockHeight, EarliestBlockTime: time.Unix(0, earliestBlockTimeNano), - MaxPeerBlockHeight: env.BlockSyncReactor.GetMaxPeerBlockHeight(), - CatchingUp: env.ConsensusReactor.WaitSync(), - TotalSyncedTime: env.BlockSyncReactor.GetTotalSyncedTime(), - RemainingTime: env.BlockSyncReactor.GetRemainingSyncTime(), + // this should start as true, if consensus + // hasn't started yet, and then flip to false + // (or true,) depending on what's actually + // happening. + CatchingUp: true, }, ValidatorInfo: validatorInfo, } + if env.ConsensusReactor != nil { + result.SyncInfo.CatchingUp = env.ConsensusReactor.WaitSync() + } + + if env.BlockSyncReactor != nil { + result.SyncInfo.MaxPeerBlockHeight = env.BlockSyncReactor.GetMaxPeerBlockHeight() + result.SyncInfo.TotalSyncedTime = env.BlockSyncReactor.GetTotalSyncedTime() + result.SyncInfo.RemainingTime = env.BlockSyncReactor.GetRemainingSyncTime() + } + if env.StateSyncMetricer != nil { result.SyncInfo.TotalSnapshots = env.StateSyncMetricer.TotalSnapshots() result.SyncInfo.ChunkProcessAvgTime = env.StateSyncMetricer.ChunkProcessAvgTime() @@ -103,6 +114,9 @@ func (env *Environment) validatorAtHeight(h int64) *types.Validator { if err != nil { return nil } + if env.ConsensusState == nil { + return nil + } if env.PubKey == nil { return nil } From 631ef7aae00bafedd64f41f0c24c71a3d0dcb0cc Mon Sep 17 00:00:00 2001 From: Sam Kleinman Date: Fri, 8 Apr 2022 17:13:10 -0400 Subject: [PATCH 10/10] consensus: add nil check to gossip routine (#8288) --- internal/consensus/reactor.go | 16 +++++++++++++--- 1 file changed, 13 insertions(+), 3 deletions(-) diff --git a/internal/consensus/reactor.go b/internal/consensus/reactor.go index c73f998d8..3020e6e16 100644 --- a/internal/consensus/reactor.go +++ b/internal/consensus/reactor.go @@ -836,10 +836,20 @@ func (r *Reactor) queryMaj23Routine(ctx context.Context, ps *PeerState, stateCh return } - rs := r.getRoundState() - prs := ps.GetRoundState() - // TODO create more reliable coppies of these + // TODO create more reliable copies of these // structures so the following go routines don't race + rs := r.getRoundState() + if rs.Votes == nil { + // if we have gotten here, we've connected to + // a peer before the state of the reactor has + // updated to the current round, so we should + // sleep for a while before we attempt to + // start gossiping the data that doesn't exist + // yet. This prevents a panic. + timer.Reset(r.state.config.PeerQueryMaj23SleepDuration) + continue + } + prs := ps.GetRoundState() wg := &sync.WaitGroup{}