tristanls / discover

Node discovery based on Kademlia DHT protocol.
MIT License
69 stars 10 forks source link

kBucket.on('ping') handler not entirely correct #1

Closed tristanls closed 11 years ago

tristanls commented 11 years ago

In Discover.prototype.register(contact) implementation, specifically in the kBucket 'ping' event handler:

// ...
            var reachedListener = function (contact) {
                if (oldContactIdsBase64.indexOf(contact.id) > -1) {
                    reachedCount++;
                    if (reachedCount == oldContactIdsBase64.length) {
                        // all contacts were reached, won't be adding new one
                        self.transport.removeListener(
                            'unreachable', unreachableListener);
                        self.transport.removeListener(
                            'reached', reachedListener);
                    }
                }
            };
// ...

The implementation uses reachedCount++ along with oldContactIdsBase64 to count off unique contacts that were reached. However, it is possible that the same contact could be 'reached' multiple times, therefore meeting the condition

if (oldContactIdsBase64.indexOf(contact.id) > -1) { // ...

and incrementing the reachCount counter multiple times, while not actually verifying that all old contacts were reached.