From a519825bf8e376ee32b46b396484b099bd9d9104 Mon Sep 17 00:00:00 2001 From: Ethan Buchman Date: Mon, 18 Jun 2018 17:08:09 -0700 Subject: [PATCH 1/7] consensus: fixes #1754 * updateToState exits early if the state isn't new, which happens after * fast syncing. This results in not sending a NewRoundStep message. The mempool * reactor depends on PeerState, which is updated by NewRoundStep * messages. If the peer never sends a NewRoundStep, the mempool reactor * will think they're behind, and never forward transactions. Note this * only happens when `create_empty_blocks = false`, because otherwise * peers will move through the consensus state and send a NewRoundStep * for a new step soon anyways. Simple fix is just to send the * NewRoundStep message during updateToState even if exit early --- consensus/state.go | 8 ++++++-- mempool/mempool.go | 2 +- mempool/reactor.go | 4 +++- types/keys.go | 3 +-- 4 files changed, 11 insertions(+), 6 deletions(-) diff --git a/consensus/state.go b/consensus/state.go index d46ec5830..3834b1515 100644 --- a/consensus/state.go +++ b/consensus/state.go @@ -460,9 +460,12 @@ func (cs *ConsensusState) updateToState(state sm.State) { // If state isn't further out than cs.state, just ignore. // This happens when SwitchToConsensus() is called in the reactor. - // We don't want to reset e.g. the Votes. + // We don't want to reset e.g. the Votes, but we still want to + // signal the new round step, because other services (eg. mempool) + // depend on having an up-to-date peer state! if !cs.state.IsEmpty() && (state.LastBlockHeight <= cs.state.LastBlockHeight) { cs.Logger.Info("Ignoring updateToState()", "newHeight", state.LastBlockHeight+1, "oldHeight", cs.state.LastBlockHeight+1) + cs.newStep() return } @@ -492,6 +495,7 @@ func (cs *ConsensusState) updateToState(state sm.State) { } else { cs.StartTime = cs.config.Commit(cs.CommitTime) } + cs.Validators = validators cs.Proposal = nil cs.ProposalBlock = nil @@ -517,7 +521,7 @@ func (cs *ConsensusState) newStep() { rs := cs.RoundStateEvent() cs.wal.Write(rs) cs.nSteps++ - // newStep is called by updateToStep in NewConsensusState before the eventBus is set! + // newStep is called by updateToState in NewConsensusState before the eventBus is set! if cs.eventBus != nil { cs.eventBus.PublishEventNewRoundStep(rs) cs.evsw.FireEvent(types.EventNewRoundStep, &cs.RoundState) diff --git a/mempool/mempool.go b/mempool/mempool.go index 5af16b3c9..bde4984b1 100644 --- a/mempool/mempool.go +++ b/mempool/mempool.go @@ -328,11 +328,11 @@ func (mem *Mempool) notifyTxsAvailable() { panic("notified txs available but mempool is empty!") } if mem.txsAvailable != nil && !mem.notifiedTxsAvailable { + // channel cap is 1, so this will send once select { case mem.txsAvailable <- mem.height + 1: default: } - mem.notifiedTxsAvailable = true } } diff --git a/mempool/reactor.go b/mempool/reactor.go index 54a3c32fe..76758704c 100644 --- a/mempool/reactor.go +++ b/mempool/reactor.go @@ -103,6 +103,7 @@ type PeerState interface { // Send new mempool txs to peer. func (memR *MempoolReactor) broadcastTxRoutine(peer p2p.Peer) { if !memR.config.Broadcast { + memR.Logger.Info("Tx broadcasting is disabled") return } @@ -129,7 +130,8 @@ func (memR *MempoolReactor) broadcastTxRoutine(peer p2p.Peer) { height := memTx.Height() if peerState_i := peer.Get(types.PeerStateKey); peerState_i != nil { peerState := peerState_i.(PeerState) - if peerState.GetHeight() < height-1 { // Allow for a lag of 1 block + peerHeight := peerState.GetHeight() + if peerHeight < height-1 { // Allow for a lag of 1 block time.Sleep(peerCatchupSleepIntervalMS * time.Millisecond) continue } diff --git a/types/keys.go b/types/keys.go index 992551191..941e82b65 100644 --- a/types/keys.go +++ b/types/keys.go @@ -2,6 +2,5 @@ package types // UNSTABLE var ( - PeerStateKey = "ConsensusReactor.peerState" - PeerMempoolChKey = "MempoolReactor.peerMempoolCh" + PeerStateKey = "ConsensusReactor.peerState" ) From 3470e5d7b3e94159fff11fbae1ef572a971f2cdc Mon Sep 17 00:00:00 2001 From: Ethan Buchman Date: Mon, 18 Jun 2018 17:15:41 -0700 Subject: [PATCH 2/7] changelog and version --- CHANGELOG.md | 8 ++++++++ version/version.go | 4 ++-- 2 files changed, 10 insertions(+), 2 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index e7f9baf2b..ed524e508 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,5 +1,13 @@ # Changelog +## 0.20.1 + +*June 18th, 2018* + +BUG FIXES + +[consensus] Fix #1754 where we don't make blocks when `create_empty_blocks=false` + ## 0.20.0 *June 6th, 2018* diff --git a/version/version.go b/version/version.go index e6c1815b5..e0e6cc2c3 100644 --- a/version/version.go +++ b/version/version.go @@ -4,13 +4,13 @@ package version const ( Maj = "0" Min = "20" - Fix = "0" + Fix = "1" ) var ( // Version is the current version of Tendermint // Must be a string because scripts like dist.sh read this file. - Version = "0.20.0" + Version = "0.20.1" // GitCommit is the current HEAD set using ldflags. GitCommit string From 6a324764acf2423062a600fa3d29628fdce3246e Mon Sep 17 00:00:00 2001 From: Ethan Buchman Date: Mon, 18 Jun 2018 17:18:35 -0700 Subject: [PATCH 3/7] fix circle --- .circleci/config.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.circleci/config.yml b/.circleci/config.yml index b2ea5883f..abd85f3c6 100644 --- a/.circleci/config.yml +++ b/.circleci/config.yml @@ -3,7 +3,7 @@ version: 2 defaults: &defaults working_directory: /go/src/github.com/tendermint/tendermint docker: - - image: circleci/golang:1.10.0 + - image: circleci/golang:1.10.3 environment: GOBIN: /tmp/workspace/bin From 4b2348f697d9b2019547693555522ae50a4ed037 Mon Sep 17 00:00:00 2001 From: Ethan Buchman Date: Mon, 18 Jun 2018 18:18:52 -0700 Subject: [PATCH 4/7] mempool: fix cache_size==0. closes #1761 --- CHANGELOG.md | 3 ++- mempool/mempool.go | 5 +++++ 2 files changed, 7 insertions(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index ed524e508..3c484fea7 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -6,7 +6,8 @@ BUG FIXES -[consensus] Fix #1754 where we don't make blocks when `create_empty_blocks=false` +- [consensus] Fix #1754 where we don't make blocks when `create_empty_blocks=false` +- [mempool] Fix #1761 where we don't process txs if `cache_size=0` ## 0.20.0 diff --git a/mempool/mempool.go b/mempool/mempool.go index bde4984b1..1ed718091 100644 --- a/mempool/mempool.go +++ b/mempool/mempool.go @@ -478,6 +478,11 @@ func (cache *txCache) Push(tx types.Tx) bool { cache.mtx.Lock() defer cache.mtx.Unlock() + // if cache size is 0, do nothing + if cache.size == 0 { + return true + } + if _, exists := cache.map_[string(tx)]; exists { return false } From 70d973016ee7310f914f72aeab4949b0587a1237 Mon Sep 17 00:00:00 2001 From: Anton Kaliaev Date: Tue, 19 Jun 2018 11:40:40 +0400 Subject: [PATCH 5/7] output msg only once during start --- mempool/reactor.go | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-) diff --git a/mempool/reactor.go b/mempool/reactor.go index 76758704c..d60852fde 100644 --- a/mempool/reactor.go +++ b/mempool/reactor.go @@ -45,6 +45,14 @@ func (memR *MempoolReactor) SetLogger(l log.Logger) { memR.Mempool.SetLogger(l) } +// OnStart implements p2p.BaseReactor. +func (memR *MempoolReactor) OnStart() error { + if !memR.config.Broadcast { + memR.Logger.Info("Tx broadcasting is disabled") + } + return nil +} + // GetChannels implements Reactor. // It returns the list of channels for this reactor. func (memR *MempoolReactor) GetChannels() []*p2p.ChannelDescriptor { @@ -103,7 +111,6 @@ type PeerState interface { // Send new mempool txs to peer. func (memR *MempoolReactor) broadcastTxRoutine(peer p2p.Peer) { if !memR.config.Broadcast { - memR.Logger.Info("Tx broadcasting is disabled") return } From 4f5492c83104347949c19fcdc8db75e5a62a4426 Mon Sep 17 00:00:00 2001 From: Anton Kaliaev Date: Tue, 19 Jun 2018 11:41:16 +0400 Subject: [PATCH 6/7] add nopTxCache (Nil Object Pattern) to better handle zero cache size --- mempool/mempool.go | 50 ++++++++++++++++++++++++++++++---------------- mempool/reactor.go | 2 +- 2 files changed, 34 insertions(+), 18 deletions(-) diff --git a/mempool/mempool.go b/mempool/mempool.go index 1ed718091..935dfaac7 100644 --- a/mempool/mempool.go +++ b/mempool/mempool.go @@ -77,7 +77,7 @@ type Mempool struct { // Keep a cache of already-seen txs. // This reduces the pressure on the proxyApp. - cache *txCache + cache txCache // A log of mempool txs wal *auto.AutoFile @@ -97,7 +97,11 @@ func NewMempool(config *cfg.MempoolConfig, proxyAppConn proxy.AppConnMempool, he recheckCursor: nil, recheckEnd: nil, logger: log.NewNopLogger(), - cache: newTxCache(config.CacheSize), + } + if config.CacheSize > 0 { + mempool.cache = newMapTxCache(config.CacheSize) + } else { + mempool.cache = nopTxCache{} } proxyAppConn.SetResponseCallback(mempool.resCb) return mempool @@ -448,41 +452,45 @@ func (memTx *mempoolTx) Height() int64 { //-------------------------------------------------------------------------------- -// txCache maintains a cache of transactions. -type txCache struct { +type txCache interface { + Reset() + Push(tx types.Tx) bool + Remove(tx types.Tx) +} + +// mapTxCache maintains a cache of transactions. +type mapTxCache struct { mtx sync.Mutex size int map_ map[string]struct{} list *list.List // to remove oldest tx when cache gets too big } -// newTxCache returns a new txCache. -func newTxCache(cacheSize int) *txCache { - return &txCache{ +var _ txCache = (*mapTxCache)(nil) + +// newMapTxCache returns a new mapTxCache. +func newMapTxCache(cacheSize int) *mapTxCache { + return &mapTxCache{ size: cacheSize, map_: make(map[string]struct{}, cacheSize), list: list.New(), } } -// Reset resets the txCache to empty. -func (cache *txCache) Reset() { +// Reset resets the cache to an empty state. +func (cache *mapTxCache) Reset() { cache.mtx.Lock() cache.map_ = make(map[string]struct{}, cache.size) cache.list.Init() cache.mtx.Unlock() } -// Push adds the given tx to the txCache. It returns false if tx is already in the cache. -func (cache *txCache) Push(tx types.Tx) bool { +// Push adds the given tx to the cache and returns true. It returns false if tx +// is already in the cache. +func (cache *mapTxCache) Push(tx types.Tx) bool { cache.mtx.Lock() defer cache.mtx.Unlock() - // if cache size is 0, do nothing - if cache.size == 0 { - return true - } - if _, exists := cache.map_[string(tx)]; exists { return false } @@ -501,8 +509,16 @@ func (cache *txCache) Push(tx types.Tx) bool { } // Remove removes the given tx from the cache. -func (cache *txCache) Remove(tx types.Tx) { +func (cache *mapTxCache) Remove(tx types.Tx) { cache.mtx.Lock() delete(cache.map_, string(tx)) cache.mtx.Unlock() } + +type nopTxCache struct{} + +var _ txCache = (*nopTxCache)(nil) + +func (nopTxCache) Reset() {} +func (nopTxCache) Push(types.Tx) bool { return true } +func (nopTxCache) Remove(types.Tx) {} diff --git a/mempool/reactor.go b/mempool/reactor.go index d60852fde..5d1f4e793 100644 --- a/mempool/reactor.go +++ b/mempool/reactor.go @@ -6,7 +6,7 @@ import ( "time" abci "github.com/tendermint/abci/types" - "github.com/tendermint/go-amino" + amino "github.com/tendermint/go-amino" "github.com/tendermint/tmlibs/clist" "github.com/tendermint/tmlibs/log" From 3d30a4294389bdaa9bb0a49a772d315b7fe89aad Mon Sep 17 00:00:00 2001 From: Anton Kaliaev Date: Tue, 19 Jun 2018 11:59:14 +0400 Subject: [PATCH 7/7] add config to issue template --- .github/ISSUE_TEMPLATE | 2 ++ 1 file changed, 2 insertions(+) diff --git a/.github/ISSUE_TEMPLATE b/.github/ISSUE_TEMPLATE index c9c1f6a01..e0edb29ff 100644 --- a/.github/ISSUE_TEMPLATE +++ b/.github/ISSUE_TEMPLATE @@ -35,6 +35,8 @@ in a case of bug. **Logs (you can paste a part showing an error or attach the whole file)**: +**Config (you can paste only the changes you've made)**: + **`/dump_consensus_state` output for consensus bugs** **Anything else do we need to know**: