diff --git a/eth/downloader/downloader.go b/eth/downloader/downloader.go index 4cd927fd5..a48b60716 100644 --- a/eth/downloader/downloader.go +++ b/eth/downloader/downloader.go @@ -30,7 +30,7 @@ var ( errLowTd = errors.New("peer's TD is too low") errBusy = errors.New("busy") errUnknownPeer = errors.New("peer's unknown or unhealthy") - errBadPeer = errors.New("action from bad peer ignored") + ErrBadPeer = errors.New("action from bad peer ignored") errTimeout = errors.New("timeout") errEmptyHashSet = errors.New("empty hash set by peer") errPeersUnavailable = errors.New("no peers available or all peers tried for block download process") @@ -376,7 +376,7 @@ func (d *Downloader) AddBlock(id string, block *types.Block, td *big.Int) error // and add the block. Otherwise just ignore it if peer == nil { glog.V(logger.Detail).Infof("Ignored block from bad peer %s\n", id) - return errBadPeer + return ErrBadPeer } peer.mu.Lock() @@ -422,10 +422,7 @@ func (d *Downloader) process(peer *peer) error { return nil } - var ( - blocks = d.queue.blocks - err error - ) + var blocks = d.queue.blocks glog.V(logger.Debug).Infof("Inserting chain with %d blocks (#%v - #%v)\n", len(blocks), blocks[0].Number(), blocks[len(blocks)-1].Number()) // Loop untill we're out of blocks @@ -435,7 +432,7 @@ func (d *Downloader) process(peer *peer) error { // processing and start requesting the `block.hash` so that it's parent and // grandparents can be requested and queued. var i int - i, err = d.insertChain(blocks[:max]) + i, err := d.insertChain(blocks[:max]) if err != nil && core.IsParentErr(err) { // Ignore the missing blocks. Handler should take care of anything that's missing. glog.V(logger.Debug).Infof("Ignored block with missing parent (%d)\n", i) @@ -447,8 +444,9 @@ func (d *Downloader) process(peer *peer) error { d.UnregisterPeer(d.activePeer) // Reset chain completely. This needs much, much improvement. // instead: check all blocks leading down to this block false block and remove it - blocks = nil - break + d.queue.blocks = nil + + return ErrBadPeer } blocks = blocks[max:] } @@ -459,7 +457,7 @@ func (d *Downloader) process(peer *peer) error { } else { d.queue.blocks = blocks } - return err + return nil } func (d *Downloader) isFetchingHashes() bool { diff --git a/eth/handler.go b/eth/handler.go index fecd71632..6cc69aa26 100644 --- a/eth/handler.go +++ b/eth/handler.go @@ -122,6 +122,12 @@ func NewProtocolManager(protocolVersion, networkId int, mux *event.TypeMux, txpo return manager } +func (pm *ProtocolManager) removePeer(peer *peer) { + pm.pmu.Lock() + defer pm.pmu.Unlock() + delete(pm.peers, peer.id) +} + func (pm *ProtocolManager) syncHandler() { // itimer is used to determine when to start ignoring `minDesiredPeerCount` itimer := time.NewTimer(peerCountTimeout) @@ -172,7 +178,10 @@ func (pm *ProtocolManager) synchronise(peer *peer) { glog.V(logger.Info).Infof("Synchronisation attempt using %s TD=%v\n", peer.id, peer.td) // Get the hashes from the peer (synchronously) err := pm.downloader.Synchronise(peer.id, peer.recentHash) - if err != nil { + if err != nil && err == downloader.ErrBadPeer { + glog.V(logger.Debug).Infoln("removed peer from peer set due to bad action") + pm.removePeer(peer) + } else if err != nil { // handle error glog.V(logger.Debug).Infoln("error downloading:", err) } @@ -214,10 +223,8 @@ func (pm *ProtocolManager) handle(p *peer) error { pm.downloader.RegisterPeer(p.id, p.recentHash, p.requestHashes, p.requestBlocks) defer func() { - pm.pmu.Lock() - defer pm.pmu.Unlock() - delete(pm.peers, p.id) pm.downloader.UnregisterPeer(p.id) + pm.removePeer(p) }() // propagate existing transactions. new transactions appearing @@ -379,16 +386,23 @@ func (self *ProtocolManager) handleMsg(p *peer) error { // if the parent does not exists we delegate to the downloader. if self.chainman.HasBlock(request.Block.ParentHash()) { if _, err := self.chainman.InsertChain(types.Blocks{request.Block}); err != nil { - // handle error + glog.V(logger.Error).Infoln("removed peer (", p.id, ") due to block error") + + self.removePeer(p) + return nil } self.BroadcastBlock(hash, request.Block) } else { // adding blocks is synchronous go func() { - // TODO check parent error err := self.downloader.AddBlock(p.id, request.Block, request.TD) - if err != nil { + if err != nil && err == downloader.ErrBadPeer { + glog.V(logger.Error).Infoln("removed peer (", p.id, ") with err:", err) + + self.removePeer(p) + return + } else if err != nil { glog.V(logger.Detail).Infoln("downloader err:", err) return }