Fix unexpected ConnectionDirection update

[ackintosh]

Description

A connection direction can unexpectedly change from Outgoing to Incoming in the scenario shown in this diagram. The diagram shows a specific case in PING/PONG. However, if we generalize this, when an Outgoing connection is established and the other node returns WHOAREYOU, the ConnectionDirection will change to Incoming at the time the new session is established. This could potentially cause some problems. e.g. IpVote doesn't work as IpVote ignores Incoming connections.

This PR makes the service to set only initial connection direction and not update it if a node already exists in the routing table.

Notes & open questions

I have tested this PR with the PING/PONG simulation (#184), and confirmed that the Outgoing connection directions are kept.

sequenceDiagram
    participant Node1
    participant Node2

    rect rgb(10, 10, 10)
    Note left of Node1: <Node1's KBucket><br>[Node2] direction:Outgoing, state:Connected
    end
    Note over Node1,Node2: Session established

    rect rgb(10, 10, 10)
    Note left of Node1: Node1 changes its IP address<br> but doesn't update its ENR.
    end 
    Note over Node1: *** Change its IP address ***

    Node1 ->> Node2: PING
    Note over Node2: No session with the new IP exists
    Node2 ->> Node1: WHOAREYOU
    Note over Node1: Establish new session
    rect rgb(0, 255, 0)
    Note left of Node1: The Outgoing direction is kept<br> when establishing a new session here. 
    end
    rect rgb(0, 255, 0)
    Note left of Node1: <Node1's KBucket><br>[Node2] direction:***Outgoing***, state:Connected
    end
    Node1 ->> Node2: Auth message
    Note over Node2: Establish one-time session with Node1's new IP
    Note over Node2: Respond the PING using the one-time session
    Node2 ->> Node1: PONG

Loading

Change checklist

• Self-review
• Documentation updates if relevant
• Tests if relevant

[ackintosh]

@AgeManning @divagant-martian Please have a look when you have time.

[AgeManning]

On a first review, it seems like we could leave the handler as it is, then in the service, we never update the direction of a node in the routing table, we only ever set the direction for a new node.

This way the code changes would be minimal, and the initial connections are kept as final (until the node is exited from the routing table).

What do you think?

[divagant-martian]

I find this change in the right direction but I missed something on the first review that changes the point of the PR

[ackintosh]

@AgeManning @divagant-martian I have tested again with the latest changes, and that was fine. 🙆‍♂️ Please have another look.

[divagant-martian]

LGTM, missing a typo

[AgeManning]

Yeah nice, looks good.

Much simpler :)