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

gossipd: don't "unmark" dying channels' updates if we receive them. #6426

Merged
merged 1 commit into from Jul 25, 2023

Conversation

rustyrussell
Copy link
Contributor

This looked like a test flake, but was real:

        l1.daemon.wait_for_log("closing soon due to the funding outpoint being spent")
    
        # We won't gossip the dead channel any more (but we still propagate node_announcement).  But connectd is not explicitly synced, so wait for "a bit".
        time.sleep(1)
>       assert len(get_gossip(l1)) == 2
E       assert 4 == 2

We can see that two channel_updates come in after we mark it dying:

gossipd: channel 103x1x0 closing soon due to the funding outpoint being spent
gossipd: REPLY WIRE_GOSSIPD_NEW_BLOCKHEIGHT_REPLY with 0 fds
022d223620a359a47ff7f7ac447c85c46c923da53389221a0054c11c1e3ca31d59-gossipd: Received channel_update for channel 103x1x0/0 now DISABLED
022d223620a359a47ff7f7ac447c85c46c923da53389221a0054c11c1e3ca31d59-gossipd: Received channel_update for channel 103x1x0/1 now DISABLED

We should keep marking channel_updates the same way.

This looked like a test flake, but was real:

```
        l1.daemon.wait_for_log("closing soon due to the funding outpoint being spent")
    
        # We won't gossip the dead channel any more (but we still propagate node_announcement).  But connectd is not explicitly synced, so wait for "a bit".
        time.sleep(1)
>       assert len(get_gossip(l1)) == 2
E       assert 4 == 2
```

We can see that two channel_updates come in *after* we mark it dying:

```
gossipd: channel 103x1x0 closing soon due to the funding outpoint being spent
gossipd: REPLY WIRE_GOSSIPD_NEW_BLOCKHEIGHT_REPLY with 0 fds
022d223620a359a47ff7f7ac447c85c46c923da53389221a0054c11c1e3ca31d59-gossipd: Received channel_update for channel 103x1x0/0 now DISABLED
022d223620a359a47ff7f7ac447c85c46c923da53389221a0054c11c1e3ca31d59-gossipd: Received channel_update for channel 103x1x0/1 now DISABLED
```

We should keep marking channel_updates the same way.

Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
@rustyrussell rustyrussell added gossip flake Known CI flakes labels Jul 22, 2023
@rustyrussell rustyrussell added this to the v23.08 milestone Jul 22, 2023
Copy link
Collaborator

@endothermicdev endothermicdev left a comment

Choose a reason for hiding this comment

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

Oh, sorry I didn't consider the incoming gossip while reviewing #6413. Of course we should persist the flag for newer gossip as well!

ACK bbe8b36

@rustyrussell rustyrussell merged commit af7e641 into ElementsProject:master Jul 25, 2023
38 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
flake Known CI flakes gossip
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants