From 4a3ee19a227100247199704e9ece6656a1eb1a0c Mon Sep 17 00:00:00 2001
From: niftynei <niftynei@gmail.com>
Date: Tue, 10 Nov 2020 18:23:21 -0600
Subject: [PATCH] connectd: Update connection list with new address

If we're already attempting to connect to a peer, we would ignore
new connection requests. This is problematic if your node has bad
connection details for the node -- you can't update it while inflight.

This patch appends new connection suggestions to the list of connections
to try.

Fixes #4154
---
 connectd/connectd.c         | 11 ++++++++++-
 tests/plugins/slow_start.py | 33 +++++++++++++++++++++++++++++++++
 tests/test_connection.py    | 24 ++++++++++++++++++++++++
 3 files changed, 67 insertions(+), 1 deletion(-)
 create mode 100755 tests/plugins/slow_start.py

diff --git a/connectd/connectd.c b/connectd/connectd.c
index 986ea2511..ae0ccdcae 100644
--- a/connectd/connectd.c
+++ b/connectd/connectd.c
@@ -1457,8 +1457,17 @@ static void try_connect_peer(struct daemon *daemon,
 		return;
 
 	/* If we're trying to connect it right now, that's OK. */
-	if (find_connecting(daemon, id))
+	if ((connect = find_connecting(daemon, id))) {
+		/* If we've been passed in new connection details
+		 * for this connection, update our addrhint + add
+		 * to addresses to check */
+		if (addrhint) {
+			connect->addrhint = tal_steal(connect, addrhint);
+			tal_arr_expand(&connect->addrs, *addrhint);
+		}
+
 		return;
+	}
 
 	/* Start an array of addresses to try. */
 	addrs = tal_arr(tmpctx, struct wireaddr_internal, 0);
diff --git a/tests/plugins/slow_start.py b/tests/plugins/slow_start.py
new file mode 100755
index 000000000..f40addbdc
--- /dev/null
+++ b/tests/plugins/slow_start.py
@@ -0,0 +1,33 @@
+#!/usr/bin/env python3
+"""This plugin is used to check that updated connection hints work properly.
+
+"""
+from pyln.client import Plugin
+
+import socket
+import time
+
+plugin = Plugin()
+
+
+@plugin.async_method('waitconn')
+def wait_connection(request, plugin):
+    sock = socket.socket(socket.AF_INET, socket.SOCK_STREAM)
+    sock.bind(('localhost', 0))
+    sock.listen(1)
+    print("listening for connections on port {}".format(sock.getsockname()[1]))
+
+    # We are a one and done socket connection!
+    conn, client_addr = sock.accept()
+    try:
+        print("connection from {}".format(client_addr))
+        time.sleep(3)
+
+    finally:
+        conn.close()
+
+    print("closing socket")
+    sock.close()
+
+
+plugin.run()
diff --git a/tests/test_connection.py b/tests/test_connection.py
index 9e733a756..a50739b3d 100644
--- a/tests/test_connection.py
+++ b/tests/test_connection.py
@@ -111,6 +111,30 @@ def test_reconnect_channel_peers(node_factory, executor):
     fut3.result(10)
 
 
+def test_connection_moved(node_factory, executor):
+    slow_start = os.path.join(os.getcwd(), 'tests/plugins/slow_start.py')
+    options = {'may_reconnect': True, 'plugin': slow_start}
+    l1, l2 = node_factory.get_nodes(2, opts=options)
+
+    # Set up the plugin to wait for a connection
+    executor.submit(l1.rpc.waitconn)
+    log = l1.daemon.wait_for_log('listening for connections')
+    match = re.search(r'on port (\d*)', log)
+    assert match and len(match.groups()) == 1
+    hang_port = match.groups()[0]
+
+    # Attempt connection
+    fut_hang = executor.submit(l1.rpc.connect, l2.info['id'],
+                               'localhost', hang_port)
+    l1.daemon.wait_for_log('connection from')
+
+    # Provide correct connection details
+    l1.rpc.connect(l2.info['id'], 'localhost', l2.port)
+
+    # If we failed to update the connection, this call will error
+    fut_hang.result(TIMEOUT)
+
+
 def test_balance(node_factory):
     l1, l2 = node_factory.line_graph(2, fundchannel=True)
     p1 = only_one(l1.rpc.getpeer(peer_id=l2.info['id'], level='info')['channels'])