lightningnetwork
lnd_test: fixing testGraphTopologyNotifications flake
This commit fixes a race condition where Alice's gossip filter would be set after the announcement generation between Bob & Carol resulting in Alice receiving no channel updates or node announcements. Now, Alice and Bob connect before the channel open between Bob and Carol. I also changed the check here:
https://github.com/lightningnetwork/lnd/blob/c4415f04008c70be20b42f85e1ceff3ef99614c2/lnd_test.go#L8609
to for numChannelUpds < 2 || numNodeAnns < 1 so it actually waits for both channel updates and node announcements. I think it should wait for an additional node announcement from Carol, but that requires a fix of #3058
This should also be changed in the same manner but would require a fix of #3058 as well: https://github.com/lightningnetwork/lnd/blob/c4415f04008c70be20b42f85e1ceff3ef99614c2/lnd_test.go#L8460
This flake is identical to #3010
I think it would be best if this wasn't merged in until #3219 was. Then I can update the code to properly check for node announcements.
@Crypt-iQ Can you update the PR description to explain why this is still needed now that #3219 is merged?
@halseth #3219 makes sure each node gets the node announcement. There are two problems present in the current testGraphTopologyNotifications itest.
- It only waits for 2 updates OR 2 node announcements before breaking out of the for loops described in the description. It needs an
||instead of&&to properly wait for both types. - There is also a race condition with Alice's gossip filter being set after announcement generation b/t Bob & Carol which causes the flake.
The reason I waited until #3219 was merged in was so that the for loops in case 1 actually receive 2 node announcements. This way the test actually represents what it says it does. Should I update the PR description w/ this?
There is also a race condition with Alice's gossip filter being set after announcement generation b/t Bob & Carol which causes the flake.
@Crypt-iQ this should actually be fine after all since Alice and Bob are already connected before each test case starts.
@wpaulino needed because Alice and Bob disconnect with each other here https://github.com/lightningnetwork/lnd/blob/2711b7c7f622d879c5991968c4be207c55c5e13a/lntest/itest/lnd_test.go#L9021-L9023
@crypt-iq, remember to re-request review from reviewers when ready
@crypt-iq, remember to re-request review from reviewers when ready
@crypt-iq, remember to re-request review from reviewers when ready
