Setting a Node involves NodeManager and NodeGraph, where NodeManager handles bucket overflow policy

[tegefaulkes]

Specification

Adding a node to a NodeGraph's bucket needs to be updated. There are 3 cases when adding a node that need to be handled

1. If the node already exists in the bucket the last updated need to be updated.
2. If the node is not already in the bucket AND the bucket has room. The node is added into the bucket.
3. If the node is not in the bucket AND the bucket is full. We need to ping the oldest node in the bucket, if the old node does not respond we drop it for the new node, otherwise we drop the new node and update the last updated for the old node.

We need to add a force flag for always add the new node for manually adding nodes.

The core of the functionality needs to be moved into the NodeManager. since the NodeGraph is purely data structure the checking of the nodes before adding them should be handled inside the NodeManager.

As implemented, the NodeGraph just stores the nodes within buckets and tracks the number of nodes in the bucket. The more complex logic is handled by the NodeManager now.

Additional context

• Testnet Deployment #326 (comment)
• Relates Testnet Deployment #326
• Relates Improve node search over k-buckets (getClosestLocalNode) #212
• Relates Investigate means of authenticating a node ID when adding to NodeGraph #322
• Relates Seed node not adding details of connecting node to its NodeGraph #344
• Relates Remove "revoked" node ID -> node address mappings in NodeGraph #150
• Relates Expunge any added "invalid" nodes from k-buckets #226
• Relates Supporting the "refresh" operation for NodeGraph buckets #345

Tasks

• 1. Remove bucket limit checking and core adding logic from NodeGraph to NodeManager.addNode method. NodeGraph is purely data-structure anything that requires complex checking or networking needs to be outside it.
• 2. Handle cases when adding a node.
  1. Node already exists.
  2. Node exists, bucket has room.
  3. Node exist, bucket is full.
• 3. Add force parameter to force adding a node if bucket is full.
• Update testing.
  • fix any tests broken by logic moving to NodeManager
  • add test for the 3 cases when adding nodes.
  • add test for force parameter

[tegefaulkes]

I'm refactoring the NodeGraph.setNode to just add a node to the bucket and increment the counter without checking. It also updates the node's lastUpdated and information without incrementing the counter if the node already exists.

However I can see an argument for having separate methods for adding and updating the node. What do you think @CMCDragonkai ?

[CMCDragonkai]

@tegefaulkes if this is a development issue, the tasks should be part of the PR that you're working on.

If it's a procedure, then the tasks are part of the issue.

[tegefaulkes]

I just noticed that the NodeGraph.setNode uses getUnixtime which seems to be to the second resolution. It just messd with the test but isn't a problem in general. Why don't we just use the date.getTime and use millisecond resolution?

[CMCDragonkai]

This functionality has to take into account the design "authenticating" nodes here: #322 (comment)

[tegefaulkes]

I'm considering this one done.