add the expect msg at the end to trigger the must not msg

rustyrussell · Aug 29, 2022 · b1677ca · b1677ca
1 parent 2c129cb
commit b1677ca
Show file tree

Hide file tree

Showing 5 changed files with 42 additions and 97 deletions.
diff --git a/docker/Dockerfile.clightning b/docker/Dockerfile.clightning
@@ -4,7 +4,6 @@ LABEL mantainer="Vincenzo Palazzo [email protected]"
 WORKDIR /work
 
 ENV BITCOIN_VERSION=23.0
-ENV CLIGHTNING_VERSION=0.11.0
 ENV DEBIAN_FRONTEND=noninteractive
 
 RUN apt-get -qq update && \
@@ -60,9 +59,6 @@ RUN git config --global user.name "John Doe" && \
 	git config --global user.email [email protected] && \
 	git clone https://github.com/ElementsProject/lightning.git && \
     cd lightning && \
-    # git checkout v$CLIGHTNING_VERSION && \
-	# fetch core lightning patch
-	git pull origin pull/5367/head && \
     poetry config virtualenvs.create false && \
     poetry install && \
     ./configure --enable-developer && \

diff --git a/lnprototest/event.py b/lnprototest/event.py
@@ -1,4 +1,5 @@
 #! /usr/bin/python3
+import logging
 import traceback
 from pyln.proto.message import Message
 import collections
@@ -123,13 +124,13 @@ def __init__(self, must_not: str, connprivkey: Optional[str] = None):
         self.must_not = must_not
 
     def matches(self, binmsg: bytes) -> bool:
-        msgnum = struct.unpack(">H", binmsg[0:2])[0]
+        msgnum = struct.unpack(">H", binmsg[:2])[0]
         msgtype = namespace().get_msgtype_by_number(msgnum)
         if msgtype:
             name = msgtype.name
         else:
             name = str(msgnum)
-
+        logging.info(f"msg {name} != from what we are looking for {self.must_not}?")
         return name == self.must_not
 
     def action(self, runner: "Runner") -> bool:

diff --git a/lnprototest/structure.py b/lnprototest/structure.py
@@ -1,5 +1,7 @@
 #! /usr/bin/python3
 import io
+import logging
+
 from .event import Event, ExpectMsg, ResolvableBool
 from .errors import SpecFileError, EventError
 from .namespace import namespace
@@ -44,6 +46,7 @@ def action(self, runner: "Runner", skip_first: bool = False) -> bool:
         super().action(runner)
         all_done = True
         for e in self.events:
+            logging.debug(f"receiving event {e}")
             if not e.enabled(runner):
                 continue
             if skip_first:

diff --git a/poetry.lock b/poetry.lock
diff --git a/tests/test_bolt2-01-close_channel.py b/tests/test_bolt2-01-close_channel.py
@@ -25,24 +25,19 @@
 
  author: https://github.com/vincenzopalazzo
 """
-from typing import Any
-
 from lnprototest import (
     ExpectMsg,
     Msg,
     Runner,
     MustNotMsg,
 )
-from lnprototest.namespace import peer_message_namespace
 from helpers import run_runner, merge_events_sequences, tx_spendable
 from lnprototest.stash import channel_id
 from spec_helper import open_and_announce_channel_helper, connect_to_node_helper
 from lnprototest.utils import BitcoinUtils, ScriptType
 
 
-def test_close_channel_shutdown_msg_normal_case_received_side(
-    runner: Runner, namespaceoverride: Any
-) -> None:
+def test_close_channel_shutdown_msg_normal_case_received_side(runner: Runner) -> None:
     """Close the channel with the other peer, and check if the
     shutdown message works in the expected way.
 
@@ -53,9 +48,7 @@ def test_close_channel_shutdown_msg_normal_case_received_side(
     | runner <- shutdown <- ln-node |
     --------------------------------
     """
-    namespaceoverride(peer_message_namespace())
-
-    # the option that the helper method feel for us
+    # the option that the helper method feels for us
     test_opts = {}
     pre_events_conn = connect_to_node_helper(
         runner, tx_spendable=tx_spendable, conn_privkey="03"
@@ -69,7 +62,7 @@ def test_close_channel_shutdown_msg_normal_case_received_side(
 
     script = BitcoinUtils.build_valid_script()
     test = [
-        # runner sent shutdown message to the ln implementation
+        # runner sent shutdown message to ln implementation
         # BOLT 2:
         # - MUST NOT send an `update_add_htlc` after a shutdown.
         Msg(
@@ -94,7 +87,7 @@ def test_close_channel_shutdown_msg_normal_case_received_side(
 
 
 def test_close_channel_shutdown_msg_wrong_script_pubkey_received_side(
-    runner: Runner, namespaceoverride: Any
+    runner: Runner,
 ) -> None:
     """Test close operation from the receiver view point, in the case when
     the sender set a wrong script pub key not specified in the spec.
@@ -103,8 +96,6 @@ def test_close_channel_shutdown_msg_wrong_script_pubkey_received_side(
     | runner <-         warning msg             <- ln-node |
     -------------------------------------------------------
     """
-    namespaceoverride(peer_message_namespace())
-
     # the option that the helper method feels for us
     test_opts = {}
     pre_events_conn = connect_to_node_helper(
@@ -126,76 +117,28 @@ def test_close_channel_shutdown_msg_wrong_script_pubkey_received_side(
             channel_id=channel_idx,
             scriptpubkey=script,
         ),
-        MustNotMsg("shutdown"),
         MustNotMsg("add_htlc"),
+        MustNotMsg("shutdown"),
         # FIXME: add support for warning messages in pyln package?
+        ExpectMsg("warning"),
     ]
     run_runner(runner, merge_events_sequences(pre=pre_events, post=test))
 
 
-def test_close_channel_allow_multiple_shutdown_msg_receive_side(
-    runner: Runner, namespaceoverride: Any
-) -> None:
-    """
-    Close operation from the receiver point of view, where the receiver will
-    receive multiple shutdown msg.
-
-    FIXME: this need to be allowed by the spec!
-     ________________________________
-    | runner -> shutdown -> ln-node |
-    | runner -> shutdown -> ln-node |
-    | runner <- shutdown <- ln-node |
-    --------------------------------
-    """
-    namespaceoverride(peer_message_namespace())
-    test_opts = {}
-    pre_events_conn = connect_to_node_helper(
-        runner, tx_spendable=tx_spendable, conn_privkey="03"
-    )
-    pre_events = open_and_announce_channel_helper(
-        runner, conn_privkey="03", opts=test_opts
-    )
-    # merge the two events
-    pre_events = merge_events_sequences(pre_events_conn, pre_events)
-    channel_idx = channel_id()
-
-    script = BitcoinUtils.build_valid_script()
-
-    test = [
-        # runner sent shutdown message to the ln implementation
-        Msg(
-            "shutdown",
-            channel_id=channel_idx,
-            scriptpubkey=script,
-        ),
-        Msg(
-            "shutdown",
-            channel_id=channel_idx,
-            scriptpubkey=script,
-        ),
-        MustNotMsg("update_add_htlc"),
-        ExpectMsg(
-            "shutdown", ignore=ExpectMsg.ignore_all_gossip, channel_id=channel_idx
-        ),
-    ]
-    run_runner(runner, merge_events_sequences(pre=pre_events, post=test))
-
-
-def test_close_channel_allow_multiple_shutdown_msg_with_diff_script_receive_side(
-    runner: Runner, namespaceoverride: Any
-) -> None:
+def test_close_channel_allow_multiple_shutdown_msg_receive_side(runner: Runner) -> None:
     """
     Close operation from the receiver point of view, where the receiver will
     receive multiple shutdown msg.
 
-    FIXME: this need to be allowed by the spec!
-     ________________________________
-    | runner -> shutdown -> ln-node |
-    | runner -> shutdown -> ln-node |
-    | runner <- shutdown <- ln-node |
-    --------------------------------
+    In This case, multiple shutdowns are not allowed by the spec:
+    # BOLT #2:
+        - MUST NOT send multiple `shutdown` messages.
+     ____________________________________
+    |   runner -> shutdown -> ln-node    |
+    |   runner -> shutdown -> ln-node    |
+    | runner <- error/warning <- ln-node |
+    -------------------------------------
     """
-    namespaceoverride(peer_message_namespace())
     test_opts = {}
     pre_events_conn = connect_to_node_helper(
         runner, tx_spendable=tx_spendable, conn_privkey="03"
@@ -224,10 +167,8 @@ def test_close_channel_allow_multiple_shutdown_msg_with_diff_script_receive_side
             scriptpubkey=script_two,
         ),
         MustNotMsg("update_add_htlc"),
-        # TODO: must be -> ExpectMsg("shutdown", channel_id=channel_idx, scriptpubkey=script_two),
-        # why the script is different
-        ExpectMsg(
-            "shutdown", ignore=ExpectMsg.ignore_all_gossip, channel_id=channel_idx
-        ),
+        MustNotMsg("shutdown"),
+        # TODO: check why message cln send here, I assume a warning
+        ExpectMsg("warning"),
     ]
     run_runner(runner, merge_events_sequences(pre=pre_events, post=test))