tests: include the test for the tests

[vincenzopalazzo]

Some implementations send an error message and, at the same time, will close the connection. Locally, this will cause a reading from a closed socket.

However, ldk now has a different life cycle for the connection, and we should receive the error message before.

 lnprototest.errors.EventError: `Peer did not disconnect` on event [{"event": "ExpectDisconnect", "file": "test_bolt2-01-open_channel.py", "pos": "345"},]
=========================== short test summary info ============================
FAILED ../lnprototest/tests/test_bolt2-01-open_channel.py::test_open_channel_opener_side_wrong_announcement_signatures
============= 1 failed, 29 passed, 17 skipped in 96.15s (0:01:36) ==============

Fixes #134
Reported-by: @Psycho-Pirate

[Copilot]

Copilot reviewed 2 out of 2 changed files in this pull request and generated no comments.

Comments suppressed due to low confidence (2)

tests/test_bolt2-01-open_channel.py:345

• The updated test now accepts either a disconnect or an error event. Consider adding explicit assertions or separate tests to verify each scenario under the new connection lifecycle.

        OneOf([ExpectDisconnect(), ExpectError()]),

tests/test_bolt1-02-unknown-messages.py:8

• [nitpick] Ensure that the replacement of TryAll with Wait aligns with the intended behavior for handling unknown messages, and verify that any timing or message-handling edge cases are adequately tested.

from lnprototest import Wait, Connect, ExpectMsg, Msg, RawMsg, Runner

[Psycho-Pirate]

I have some questions.

1. Why are we using Sequence instead of OneOf? Shouldn't this be a OneOf list of acceptable responses? That way we might not have to specify is_cln?
2. Instead of ExpectMsg("error") maybe we can use ExpectError().

I was going through the BOLT specification and found

The recipient:
if signature is incorrect OR non-compliant with LOW-S-standard rulehttps://github.com/bitcoin/bitcoin/pull/6769:
MUST send a warning and close the connection, or send an error and fail the channel.

We are checking for Disconnect(closing connection) and Error, maybe we should also check channel closure?

[vincenzopalazzo]

Shouldn't this be a OneOf list of acceptable responses?

Yes, but now the OneOf event will read through the buffer and consume the buffer, so the next event will read from an incorrect buffer. The Sequence allows for disabling the execution for each run.

There could be smarter way to use OneOf, but I am working on a new API that allows a better way of writing tests, so I will stay with what we have!

We are checking for Disconnect(closing connection) and Error, maybe we should also check channel closure?

Right, but we do not have anyone who is now sending warnings. When we have them, we should add the warning there.

[vincenzopalazzo]

Ah probably the ExpectDisconnect when find the connection ready, and it is able to consume the buffer can stash the info inside the Runner, and then ExpectMsg check for stashed message :/ but this is so complicated.

We can track this with an issue? In this way we can track it?