Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Connectd: more cleanups after #4984 #5005

Merged

Conversation

rustyrussell
Copy link
Contributor

@rustyrussell rustyrussell commented Jan 12, 2022

Cleanups, now #4984 is merged.

Changelog-None

@rustyrussell rustyrussell added this to the v0.11 milestone Jan 12, 2022
This was put in late 2019, and @t-bast says Eclair doesn't ignore their
errors and has had no issues.

It also conflicts with lightning/bolts#932
which suggests you *should* fail when you receive an error.

Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
OK, now this test makes more sense!  Now we don't ignore errors, we
*will* drop to chain if we reconnect after one side has dropped to
chain.

Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
connectd does this internally now using ccan/io, with appropriate
credit for ZmnSCPxj who wrote this code in the first place.

Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
check-dbstmts was just running the normal pytest, AFAICT:

```
export TEST_CHECK_DBSTMTS=0
+ TEST_CHECK_DBSTMTS=0
```

Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
This would never trigger, since test was backward.

Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
Copy link
Member

@cdecker cdecker left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ACK b17adde

Copy link
Collaborator

@vincenzopalazzo vincenzopalazzo left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

failure looks unrelated,

ACK b17adde

lightningd would race with the subd destructor to do the waitpid(),
resulting in UNUSUAL log messages, but also us missing if a plugin
was killed via a signal.

We can also get rid of the gratuitous waitpid() in test_subdaemons.

Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
An "err" is only returned if the channel_update is malformed: more common
is that it's fine, but we don't know the scid.

Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
```
        l1.rpc.disconnect(l2.info['id'], force=True)
        l1.rpc.connect(l2.info['id'], 'localhost', l2.port)
    
>       l1.daemon.wait_for_log('option_static_remotekey enabled at 2/2')

tests/test_connection.py:3653: 
```

If l2's channeld gets killed (due to reconnect) before it tells
lightningd it got the revoke_and_ack it will need a retransmission
*again*.

This makes the test more robust, and does more checks too. 

Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
@cdecker
Copy link
Member

cdecker commented Jan 24, 2022

ACK 267d4d6

@rustyrussell rustyrussell merged commit 16dd091 into ElementsProject:master Jan 24, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants