From 2650459012efe859f2da7e6bbc27b912c96a3a7a Mon Sep 17 00:00:00 2001 From: Neil Booth Date: Sun, 26 Mar 2017 11:36:19 +0900 Subject: [PATCH] Peer discovery fixes Change last_connect to mean last connection as its name implies, not last connection that wasn't bad. Keep bad peers around for 3 tries. Improve diagnostic --- server/peers.py | 114 ++++++++++++++++++++++++++---------------------- 1 file changed, 62 insertions(+), 52 deletions(-) diff --git a/server/peers.py b/server/peers.py index 2bfe546..0c70fc0 100644 --- a/server/peers.py +++ b/server/peers.py @@ -56,7 +56,9 @@ class PeerSession(JSONSession): self.peer = peer self.peer_mgr = peer_mgr self.kind = kind + self.failed = False self.bad = False + self.remote_peers = None self.log_prefix = '[{}] '.format(self.peer) async def wait_on_items(self): @@ -92,20 +94,66 @@ class PeerSession(JSONSession): def on_peers_subscribe(self, result, error): '''Handle the response to the peers.subcribe message.''' if error: - self.bad = True + self.failed = True self.log_error('server.peers.subscribe: {}'.format(error)) else: - self.check_remote_peers(result) + self.remote_peers = result self.close_if_done() - def check_remote_peers(self, updates): + def on_add_peer(self, result, error): + '''Handle the response to the add_peer message.''' + self.close_if_done() + + def on_features(self, features, error): + # Several peers don't implement this. If they do, check they are + # the same network with the genesis hash. + if not error and isinstance(features, dict): + our_hash = self.peer_mgr.env.coin.GENESIS_HASH + if our_hash != features.get('genesis_hash'): + self.bad = True + self.log_warning('incorrect genesis hash') + else: + self.peer.update_features(features) + self.close_if_done() + + def on_headers(self, result, error): + '''Handle the response to the version message.''' + if error: + self.failed = True + self.log_error('blockchain.headers.subscribe returned an error') + elif not isinstance(result, dict): + self.bad = True + self.log_error('bad blockchain.headers.subscribe response') + else: + our_height = self.peer_mgr.controller.bp.db_height + their_height = result.get('block_height') + if not isinstance(their_height, int): + self.log_warning('invalid height {}'.format(their_height)) + self.bad = True + elif abs(our_height - their_height) > 5: + self.log_warning('bad height {:,d} (ours: {:,d})' + .format(their_height, our_height)) + self.bad = True + self.close_if_done() + + def on_version(self, result, error): + '''Handle the response to the version message.''' + if error: + self.failed = True + self.log_error('server.version returned an error') + elif isinstance(result, str): + self.peer.server_version = result + self.peer.features['server_version'] = result + self.close_if_done() + + def check_remote_peers(self): '''When a peer gives us a peer update. Each update is expected to be of the form: [ip_addr, hostname, ['v1.0', 't51001', 's51002']] ''' try: - real_names = [' '.join([u[1]] + u[2]) for u in updates] + real_names = [' '.join([u[1]] + u[2]) for u in self.remote_peers] peers = [Peer.from_real_name(real_name, str(self.peer)) for real_name in real_names] except Exception: @@ -130,61 +178,21 @@ class PeerSession(JSONSession): self.log_info('registering ourself with server.add_peer') self.send_request(self.on_add_peer, 'server.add_peer', [my.features]) - def on_add_peer(self, result, error): - '''Handle the response to the add_peer message.''' - self.close_if_done() - - def on_features(self, features, error): - # Several peers don't implement this. If they do, check they are - # the same network with the genesis hash. - if not error and isinstance(features, dict): - our_hash = self.peer_mgr.env.coin.GENESIS_HASH - if our_hash != features.get('genesis_hash'): - self.bad = True - self.log_warning('incorrect genesis hash') - else: - self.peer.update_features(features) - self.close_if_done() - - def on_headers(self, result, error): - '''Handle the response to the version message.''' - if error: - self.bad = True - self.log_error('blockchain.headers.subscribe returned an error') - elif not isinstance(result, dict): - self.bad = True - self.log_error('bad blockchain.headers.subscribe response') - else: - our_height = self.peer_mgr.controller.bp.db_height - their_height = result.get('block_height') - is_good = (isinstance(their_height, int) and - abs(our_height - their_height) <= 5) - if not is_good: - self.bad = True - self.log_warning('bad height {}'.format(their_height)) - self.close_if_done() - - def on_version(self, result, error): - '''Handle the response to the version message.''' - if error: - self.bad = True - self.log_error('server.version returned an error') - elif isinstance(result, str): - self.peer.server_version = result - self.peer.features['server_version'] = result - self.close_if_done() - def close_if_done(self): if not self.has_pending_requests(): if self.bad: self.peer.mark_bad() - self.peer_mgr.set_connection_status(self.peer, not self.bad) + elif self.remote_peers: + self.check_remote_peers() + self.peer.last_connect = time.time() + is_good = not (self.failed or self.bad) + self.peer_mgr.set_connection_status(self.peer, is_good) if self.peer.is_tor: how = 'via {} over Tor'.format(self.kind) else: how = 'via {} at {}'.format(self.kind, self.peer_addr(anon=False)) - status = 'failed to verify' if self.bad else 'verified' + status = 'verified' if is_good else 'failed to verify' elapsed = time.time() - self.peer.last_try self.log_info('{} {} in {:.1f}s'.format(status, how, elapsed)) self.close_connection() @@ -536,7 +544,6 @@ class PeerManager(util.LoggedClass): '''Called when a connection succeeded or failed.''' if good: peer.try_count = 0 - peer.last_connect = time.time() peer.source = 'peer' # Remove matching IP addresses for match in peer.matches(self.peers): @@ -547,7 +554,10 @@ class PeerManager(util.LoggedClass): def maybe_forget_peer(self, peer): '''Forget the peer if appropriate, e.g. long-term unreachable.''' - try_limit = 10 if peer.last_connect else 3 + if peer.last_connect and not peer.bad: + try_limit = 10 + else: + try_limit = 3 forget = peer.try_count >= try_limit if forget: