From e8e7c4719f0923789f08fc923378f2121e4a26ff Mon Sep 17 00:00:00 2001
From: Rusty Russell <rusty@rustcorp.com.au>
Date: Wed, 8 Sep 2021 06:26:06 +0930
Subject: [PATCH] pyln-client: fix mypy warnings, fix and test deletion of a
 channel.

This only happens when a deletion is added by a running gossipd, so
we put a deletion at the end of the store to test it.

mypy noticed that this code was nonsensical, so clearly untested.

The testing noticed that making a nodeid from a string was also buggy.

Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
---
 contrib/pyln-client/pyln/client/gossmap.py    |  24 +++++++++---------
 .../tests/data/gossip_store-part2.xz          | Bin 18176 -> 18176 bytes
 contrib/pyln-client/tests/test_gossmap.py     |   9 +++++--
 3 files changed, 19 insertions(+), 14 deletions(-)

diff --git a/contrib/pyln-client/pyln/client/gossmap.py b/contrib/pyln-client/pyln/client/gossmap.py
index 4215090be..71379d438 100755
--- a/contrib/pyln-client/pyln/client/gossmap.py
+++ b/contrib/pyln-client/pyln/client/gossmap.py
@@ -54,7 +54,7 @@ class GossmapHalfchannel(object):
 
 
 class GossmapNodeId(object):
-    def __init__(self, buf: bytes):
+    def __init__(self, buf: Union[bytes, str]):
         if isinstance(buf, str):
             buf = bytes.fromhex(buf)
         if len(buf) != 33 or (buf[0] != 2 and buf[0] != 3):
@@ -84,7 +84,7 @@ class GossmapNodeId(object):
     def from_str(cls, s: str):
         if s.startswith('0x'):
             s = s[2:]
-        if len(s) != 67:
+        if len(s) != 66:
             raise ValueError(f"{s} is not a valid hexstring of a node_id")
         return cls(bytes.fromhex(s))
 
@@ -140,7 +140,7 @@ class GossmapNode(object):
 
 .channels is a list of the GossmapChannels attached to this node.
 """
-    def __init__(self, node_id: GossmapNodeId):
+    def __init__(self, node_id: Union[GossmapNodeId, bytes, str]):
         if isinstance(node_id, bytes) or isinstance(node_id, str):
             node_id = GossmapNodeId(node_id)
         self.announce_fields: Optional[Dict[str, Any]] = None
@@ -194,15 +194,14 @@ class Gossmap(object):
 
     def _del_channel(self, scid: ShortChannelId):
         c = self.channels[scid]
-        n1 = self.nodes[c.node1_id]
-        n2 = self.nodes[c.node2_id]
-        n1.channels.remove(c)
-        n2.channels.remove(c)
+        del self.channels[scid]
+        c.node1.channels.remove(c)
+        c.node2.channels.remove(c)
         # Beware self-channels n1-n1!
-        if len(n1.channels) == 0 and n1 != n2:
-            del self.nodes[c.node1_id]
-        if len(n2.channels):
-            del self.nodes[c.node2_id]
+        if len(c.node1.channels) == 0 and c.node1 != c.node2:
+            del self.nodes[c.node1.node_id]
+        if len(c.node2.channels) == 0:
+            del self.nodes[c.node2.node_id]
 
     def _add_channel(self, rec: bytes, off: int, is_private: bool):
         fields = channel_announcement.read(io.BytesIO(rec[2:]), {})
@@ -252,7 +251,8 @@ class Gossmap(object):
         assert False
 
     def _remove_channel_by_deletemsg(self, rec: bytes):
-        scid, = struct.unpack(">Q", rec[2:])
+        scidint, = struct.unpack(">Q", rec[2:])
+        scid = ShortChannelId.from_int(scidint)
         # It might have already been deleted when we skipped it.
         if scid in self.channels:
             self._del_channel(scid)
diff --git a/contrib/pyln-client/tests/data/gossip_store-part2.xz b/contrib/pyln-client/tests/data/gossip_store-part2.xz
index 9790e4df58fd52cce80e28543b7528cdba71572c..7bf94f3c05103ee58fe0c7fc4e62c2c289c118bf 100644
GIT binary patch
delta 55
zcmZqZV{GVSoFKz^V597AHw~{V-HrQCUSik~J2hR?;>l44#yh=?AC@qtD4vercqIKd
L69W*iL`DGsl*|@c

delta 55
zcmZqZV{GVSoFK!vccbiXH;u2-hZZmZ!8FYV4@tY-iVTc5dl^40VZ6S9jep~j^xsS%
INtVbc04UuO)c^nh

diff --git a/contrib/pyln-client/tests/test_gossmap.py b/contrib/pyln-client/tests/test_gossmap.py
index c4e625011..6591ba976 100644
--- a/contrib/pyln-client/tests/test_gossmap.py
+++ b/contrib/pyln-client/tests/test_gossmap.py
@@ -28,6 +28,12 @@ def test_gossmap(tmp_path):
 
     g.refresh()
 
+    # This actually deletes a channel, which deletes a node.
+    assert g.get_channel("686386x1093x1") is None
+    assert g.get_node('029deaf9d2fba868fe0a124050f0a13e021519a12f41bea34f391fe7533fb3166d') is None
+    # The other node is untouched
+    assert g.get_node('02e0af3c70bf42343316513e54683b10c01d906c04a05dfcd9479b90d7beed9129')
+
     # It will notice the new ones.
     assert chans < len(g.channels)
     assert nodes < len(g.nodes)
@@ -38,9 +44,8 @@ def test_gossmap(tmp_path):
     assert set(g.nodes.keys()) == set(g2.nodes.keys())
 
     # Check some details
-    channel1 = g.get_channel("686386x1093x1")
     channel2 = g.get_channel("686200x1137x0")
-    assert channel1.satoshis == 1000000
+    assert g.get_channel("686386x1093x1") is None
     assert channel2.satoshis == 3000000