Skip to content

Commit

Permalink
Fix critical unintentional variable name reuse
Browse files Browse the repository at this point in the history
In `devp2p.discovery.recv_neighbours()` the variable `node` (ostensibly
containing the sender node) was reused as a temporary variable inside a 
for loop. 

Depending on the order of the `neighbours_lst` this lead in many cases
to neighbour information being attributed to the wrong sender node.

Thanks to @konradkonrad for the pair debugging :)

Refs: HydraChain/hydrachain#73
  • Loading branch information
ulope committed Jun 23, 2016
1 parent 8eef23e commit c492b00
Showing 1 changed file with 7 additions and 9 deletions.
16 changes: 7 additions & 9 deletions devp2p/discovery.py
Original file line number Diff line number Diff line change
Expand Up @@ -489,24 +489,22 @@ def send_neighbours(self, node, neighbours):
self.send(node, message)

def recv_neighbours(self, nodeid, payload, mdc):
node = self.get_node(nodeid)
remote = self.get_node(nodeid)
assert len(payload) == 1
neighbours_lst = payload[0]
assert isinstance(neighbours_lst, list)
log.debug('<<< neigbours', remoteid=node, count=len(neighbours_lst))
neighbours = []
neighbours_set = set(tuple(x) for x in neighbours_lst)
if len(neighbours_set) < len(neighbours_lst):
log.warn('received duplicates')

for n in neighbours_set:
n = list(n)
neighbours = []
for n in neighbours_lst:
nodeid = n.pop()
address = Address.from_endpoint(*n)
node = self.get_node(nodeid, address)
assert node.address
neighbours.append(node)
self.kademlia.recv_neighbours(node, neighbours)

log.debug('<<< neighbours', remoteid=remote, local=self.this_node, neighbours=neighbours,
count=len(neighbours_lst))
self.kademlia.recv_neighbours(remote, neighbours)


class NodeDiscovery(BaseService, DiscoveryProtocolTransport):
Expand Down

1 comment on commit c492b00

@heikoheiko
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

awesome!

Please sign in to comment.