diff --git a/CHANGELOG.md b/CHANGELOG.md index d7675d296..3fb471a00 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,5 +1,17 @@ # Changelog +## v0.27.2 + +*December 16th, 2018* + +### IMPROVEMENTS: + +- [node] [\#3025](https://github.com/tendermint/tendermint/issues/3025) Validate NodeInfo addresses on startup. + +### BUG FIXES: + +- [p2p] [\#3025](https://github.com/tendermint/tendermint/pull/3025) Revert to using defers in addrbook. Fixes deadlocks in pex and consensus upon invalid ExternalAddr/ListenAddr configuration. + ## v0.27.1 *December 15th, 2018* diff --git a/CHANGELOG_PENDING.md b/CHANGELOG_PENDING.md index e9a1d52f8..335c6732c 100644 --- a/CHANGELOG_PENDING.md +++ b/CHANGELOG_PENDING.md @@ -1,4 +1,4 @@ -## v0.27.2 +## v0.27.3 *TBD* @@ -21,3 +21,4 @@ Special thanks to external contributors on this release: ### IMPROVEMENTS: ### BUG FIXES: + diff --git a/node/node.go b/node/node.go index b56a3594e..993f1cd1c 100644 --- a/node/node.go +++ b/node/node.go @@ -348,20 +348,21 @@ func NewNode(config *cfg.Config, indexerService := txindex.NewIndexerService(txIndexer, eventBus) indexerService.SetLogger(logger.With("module", "txindex")) - var ( - p2pLogger = logger.With("module", "p2p") - nodeInfo = makeNodeInfo( - config, - nodeKey.ID(), - txIndexer, - genDoc.ChainID, - p2p.NewProtocolVersion( - version.P2PProtocol, // global - state.Version.Consensus.Block, - state.Version.Consensus.App, - ), - ) + p2pLogger := logger.With("module", "p2p") + nodeInfo, err := makeNodeInfo( + config, + nodeKey.ID(), + txIndexer, + genDoc.ChainID, + p2p.NewProtocolVersion( + version.P2PProtocol, // global + state.Version.Consensus.Block, + state.Version.Consensus.App, + ), ) + if err != nil { + return nil, err + } // Setup Transport. var ( @@ -782,7 +783,7 @@ func makeNodeInfo( txIndexer txindex.TxIndexer, chainID string, protocolVersion p2p.ProtocolVersion, -) p2p.NodeInfo { +) (p2p.NodeInfo, error) { txIndexerStatus := "on" if _, ok := txIndexer.(*null.TxIndex); ok { txIndexerStatus = "off" @@ -817,7 +818,8 @@ func makeNodeInfo( nodeInfo.ListenAddr = lAddr - return nodeInfo + err := nodeInfo.Validate() + return nodeInfo, err } //------------------------------------------------------------------------------ diff --git a/p2p/netaddress.go b/p2p/netaddress.go index f60271bcc..5534ded98 100644 --- a/p2p/netaddress.go +++ b/p2p/netaddress.go @@ -175,6 +175,9 @@ func (na *NetAddress) Same(other interface{}) bool { // String representation: @: func (na *NetAddress) String() string { + if na == nil { + return "" + } if na.str == "" { addrStr := na.DialString() if na.ID != "" { @@ -186,6 +189,9 @@ func (na *NetAddress) String() string { } func (na *NetAddress) DialString() string { + if na == nil { + return "" + } return net.JoinHostPort( na.IP.String(), strconv.FormatUint(uint64(na.Port), 10), diff --git a/p2p/node_info.go b/p2p/node_info.go index 99daf7c43..3e02e9c18 100644 --- a/p2p/node_info.go +++ b/p2p/node_info.go @@ -36,7 +36,7 @@ type nodeInfoAddress interface { // nodeInfoTransport validates a nodeInfo and checks // our compatibility with it. It's for use in the handshake. type nodeInfoTransport interface { - ValidateBasic() error + Validate() error CompatibleWith(other NodeInfo) error } @@ -103,7 +103,7 @@ func (info DefaultNodeInfo) ID() ID { return info.ID_ } -// ValidateBasic checks the self-reported DefaultNodeInfo is safe. +// Validate checks the self-reported DefaultNodeInfo is safe. // It returns an error if there // are too many Channels, if there are any duplicate Channels, // if the ListenAddr is malformed, or if the ListenAddr is a host name @@ -116,7 +116,7 @@ func (info DefaultNodeInfo) ID() ID { // International clients could then use punycode (or we could use // url-encoding), and we just need to be careful with how we handle that in our // clients. (e.g. off by default). -func (info DefaultNodeInfo) ValidateBasic() error { +func (info DefaultNodeInfo) Validate() error { // ID is already validated. diff --git a/p2p/node_info_test.go b/p2p/node_info_test.go index c9a72dbc2..19567d2bf 100644 --- a/p2p/node_info_test.go +++ b/p2p/node_info_test.go @@ -12,7 +12,7 @@ func TestNodeInfoValidate(t *testing.T) { // empty fails ni := DefaultNodeInfo{} - assert.Error(t, ni.ValidateBasic()) + assert.Error(t, ni.Validate()) channels := make([]byte, maxNumChannels) for i := 0; i < maxNumChannels; i++ { @@ -68,13 +68,13 @@ func TestNodeInfoValidate(t *testing.T) { // test case passes ni = testNodeInfo(nodeKey.ID(), name).(DefaultNodeInfo) ni.Channels = channels - assert.NoError(t, ni.ValidateBasic()) + assert.NoError(t, ni.Validate()) for _, tc := range testCases { ni := testNodeInfo(nodeKey.ID(), name).(DefaultNodeInfo) ni.Channels = channels tc.malleateNodeInfo(&ni) - err := ni.ValidateBasic() + err := ni.Validate() if tc.expectErr { assert.Error(t, err, tc.testName) } else { diff --git a/p2p/pex/addrbook.go b/p2p/pex/addrbook.go index d8954f230..cfeefb343 100644 --- a/p2p/pex/addrbook.go +++ b/p2p/pex/addrbook.go @@ -162,26 +162,29 @@ func (a *addrBook) FilePath() string { // AddOurAddress one of our addresses. func (a *addrBook) AddOurAddress(addr *p2p.NetAddress) { - a.Logger.Info("Add our address to book", "addr", addr) a.mtx.Lock() + defer a.mtx.Unlock() + + a.Logger.Info("Add our address to book", "addr", addr) a.ourAddrs[addr.String()] = struct{}{} - a.mtx.Unlock() } // OurAddress returns true if it is our address. func (a *addrBook) OurAddress(addr *p2p.NetAddress) bool { a.mtx.Lock() + defer a.mtx.Unlock() + _, ok := a.ourAddrs[addr.String()] - a.mtx.Unlock() return ok } func (a *addrBook) AddPrivateIDs(IDs []string) { a.mtx.Lock() + defer a.mtx.Unlock() + for _, id := range IDs { a.privateIDs[p2p.ID(id)] = struct{}{} } - a.mtx.Unlock() } // AddAddress implements AddrBook @@ -191,6 +194,7 @@ func (a *addrBook) AddPrivateIDs(IDs []string) { func (a *addrBook) AddAddress(addr *p2p.NetAddress, src *p2p.NetAddress) error { a.mtx.Lock() defer a.mtx.Unlock() + return a.addAddress(addr, src) } @@ -198,6 +202,7 @@ func (a *addrBook) AddAddress(addr *p2p.NetAddress, src *p2p.NetAddress) error { func (a *addrBook) RemoveAddress(addr *p2p.NetAddress) { a.mtx.Lock() defer a.mtx.Unlock() + ka := a.addrLookup[addr.ID] if ka == nil { return @@ -211,14 +216,16 @@ func (a *addrBook) RemoveAddress(addr *p2p.NetAddress) { func (a *addrBook) IsGood(addr *p2p.NetAddress) bool { a.mtx.Lock() defer a.mtx.Unlock() + return a.addrLookup[addr.ID].isOld() } // HasAddress returns true if the address is in the book. func (a *addrBook) HasAddress(addr *p2p.NetAddress) bool { a.mtx.Lock() + defer a.mtx.Unlock() + ka := a.addrLookup[addr.ID] - a.mtx.Unlock() return ka != nil } @@ -292,6 +299,7 @@ func (a *addrBook) PickAddress(biasTowardsNewAddrs int) *p2p.NetAddress { func (a *addrBook) MarkGood(addr *p2p.NetAddress) { a.mtx.Lock() defer a.mtx.Unlock() + ka := a.addrLookup[addr.ID] if ka == nil { return @@ -306,6 +314,7 @@ func (a *addrBook) MarkGood(addr *p2p.NetAddress) { func (a *addrBook) MarkAttempt(addr *p2p.NetAddress) { a.mtx.Lock() defer a.mtx.Unlock() + ka := a.addrLookup[addr.ID] if ka == nil { return @@ -461,12 +470,13 @@ ADDRS_LOOP: // ListOfKnownAddresses returns the new and old addresses. func (a *addrBook) ListOfKnownAddresses() []*knownAddress { - addrs := []*knownAddress{} a.mtx.Lock() + defer a.mtx.Unlock() + + addrs := []*knownAddress{} for _, addr := range a.addrLookup { addrs = append(addrs, addr.copy()) } - a.mtx.Unlock() return addrs } @@ -476,6 +486,7 @@ func (a *addrBook) ListOfKnownAddresses() []*knownAddress { func (a *addrBook) Size() int { a.mtx.Lock() defer a.mtx.Unlock() + return a.size() } diff --git a/p2p/test_util.go b/p2p/test_util.go index 44f27be40..ea788b79f 100644 --- a/p2p/test_util.go +++ b/p2p/test_util.go @@ -24,7 +24,7 @@ type mockNodeInfo struct { func (ni mockNodeInfo) ID() ID { return ni.addr.ID } func (ni mockNodeInfo) NetAddress() *NetAddress { return ni.addr } -func (ni mockNodeInfo) ValidateBasic() error { return nil } +func (ni mockNodeInfo) Validate() error { return nil } func (ni mockNodeInfo) CompatibleWith(other NodeInfo) error { return nil } func AddPeerToSwitch(sw *Switch, peer Peer) { diff --git a/p2p/transport.go b/p2p/transport.go index b16db54db..69fab312c 100644 --- a/p2p/transport.go +++ b/p2p/transport.go @@ -350,7 +350,7 @@ func (mt *MultiplexTransport) upgrade( } } - if err := nodeInfo.ValidateBasic(); err != nil { + if err := nodeInfo.Validate(); err != nil { return nil, nil, ErrRejected{ conn: c, err: err, diff --git a/version/version.go b/version/version.go index 3c1e2f319..caf6c73f3 100644 --- a/version/version.go +++ b/version/version.go @@ -18,7 +18,7 @@ const ( // TMCoreSemVer is the current version of Tendermint Core. // It's the Semantic Version of the software. // Must be a string because scripts like dist.sh read this file. - TMCoreSemVer = "0.27.1" + TMCoreSemVer = "0.27.2" // ABCISemVer is the semantic version of the ABCI library ABCISemVer = "0.15.0"