Skip to content

pytest: Fix flaky test_closing_id test#1473

Merged
rustyrussell merged 1 commit into
ElementsProject:masterfrom
cdecker:test_closing_id-flaky
May 8, 2018
Merged

pytest: Fix flaky test_closing_id test#1473
rustyrussell merged 1 commit into
ElementsProject:masterfrom
cdecker:test_closing_id-flaky

Conversation

@cdecker
Copy link
Copy Markdown
Member

@cdecker cdecker commented May 7, 2018

It simply wasn't waiting for us to register the peer before attempting to open a
connection. Moved into a separate test to be able rerun multiple times.

It simply wasn't waiting for us to register the peer before attempting to open a
connection. Moved into a separate test to be able rerun multiple times.

Signed-off-by: Christian Decker <decker.christian@gmail.com>
@cdecker cdecker added the testing label May 7, 2018
@cdecker cdecker self-assigned this May 7, 2018
@cdecker cdecker requested review from ZmnSCPxj and rustyrussell May 7, 2018 20:34
@ZmnSCPxj
Copy link
Copy Markdown
Contributor

ZmnSCPxj commented May 7, 2018

I wonder if this is good solution, though. If we use an automated system (i.e. a program that manages channels for you), and it decides to channel to a completely new peer, then the automated program would have to look into our c-lightning logs? It seems better if an automated system could connect and the fundchannel as soon as connect returns.

@rustyrussell
Copy link
Copy Markdown
Contributor

In this case, I'm pretty sure the problem is that l1 has it closed, but not l2 yet. That's not a real problem, AFAICT, just a test artifact.

@rustyrussell rustyrussell merged commit 104645b into ElementsProject:master May 8, 2018
@ZmnSCPxj
Copy link
Copy Markdown
Contributor

ZmnSCPxj commented May 8, 2018

But it seems to imply that the sequence close connect fundchannel is unsafe for automated programs. One way this may be relevant is for example if a user makes a program that ensures a channel with a particular node gets refilled when it is depleted (e.g. the end user is a frequent customer of a merchant, for example). A simple way to do that, if the user has onchain funds, is precisely the close connect fundchannel sequence in the test.

@cdecker
Copy link
Copy Markdown
Member Author

cdecker commented May 8, 2018

We can check locally that the connection was closed, but not remotely, so there is little we can do but retry.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants