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

No more gossip flood #2700

Merged

Conversation

@rustyrussell
Copy link
Contributor

commented Jun 3, 2019

This gets us a long way towards #1879; it can do with further refinement, but it's the minimal fix (important for the release , since upgrading will discard the store!).

It's built on top of #2685 and #2644 so review after 724b0cf (merged)

@rustyrussell rustyrussell requested a review from cdecker as a code owner Jun 3, 2019

@rustyrussell rustyrussell added the gossip label Jun 3, 2019

@rustyrussell rustyrussell added this to the 0.7.1 milestone Jun 3, 2019

@rustyrussell rustyrussell force-pushed the rustyrussell:no-more-gossip-flood branch from f075bca to 73d62a1 Jun 3, 2019

@darosior
Copy link
Collaborator

left a comment

Tested, works well

}

out:
if (taken(msg))
tal_free(msg);
}

/* takes iff returns true */

This comment has been minimized.

Copy link
@darosior

darosior Jun 3, 2019

Collaborator

nits: tyfpo ?

This comment has been minimized.

Copy link
@niftynei

niftynei Jun 5, 2019

Collaborator

iff is shorthand for 'if and only if'

This comment has been minimized.

Copy link
@darosior

darosior Jun 5, 2019

Collaborator

Thanks didnt know that 😅

@rustyrussell rustyrussell force-pushed the rustyrussell:no-more-gossip-flood branch from 73d62a1 to eff9089 Jun 6, 2019

@rustyrussell

This comment has been minimized.

Copy link
Contributor Author

commented Jun 6, 2019

Rebased, no changes.

@cdecker
Copy link
Member

left a comment

ACK eff9089

else
glevel = GOSSIP_MEDIUM;

while (gossip_levels[glevel] > gossip_level_targets[glevel])

This comment has been minimized.

Copy link
@cdecker

cdecker Jun 11, 2019

Member

Shouldn't this be >=?

This comment has been minimized.

Copy link
@rustyrussell

rustyrussell Jun 11, 2019

Author Contributor

Yes, good point. We also don't provide any insight into what gossip level is for each peer, which would be nice (but means a round-trip to gossipd for listpeers...).

Fixed

rustyrussell added some commits Jun 11, 2019

gossipd: note if loaded store seems reasonably up-to-date.
If not, we can ask peers for full gossip (for now we just set a flag).

Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
gossipd: prepare for internally-generated short-channel-id queries.
Up until now we only generated these in dev mode for testing.  Hoist
into common code, turn counter into a flag (we're only allowed one!)
and note if query is internal or not.

Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
gossipd: APIs return more information about routing message handling.
In particular, we'll need to know the short_channel_id if a
channel_update is unknown (implies we're missing a channel), and whether
processing a pending channel_announcement was successful (implies that
the channel was real).

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

@rustyrussell rustyrussell force-pushed the rustyrussell:no-more-gossip-flood branch from eff9089 to 7afb314 Jun 11, 2019

rustyrussell added some commits Jun 11, 2019

gossipd: query unknown short_channel_ids, note if they were really mi…
…ssing.

The first sign that we're missing gossip is that we get a channel_update
for an unknown channel.  The peer might be wrong (or lying), but if it turns
out to be a real channel, we were definitely missing something.

This patch does two things: queries when we get an unknown channel_update,
and then notes that a channel_announcement was from such an update when
it's finally processed.

Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
gossipd: control gossip level so we don't get flooded by peers.
We seek a certain number of peers at each level of gossip; 3 "flood"
if we're missing gossip, 2 at 24 hours past to catch recent gossip, and
8 with current gossip.  The rest are given a filter which causes them
not to gossip to us at all.

Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
gossipd: reset gossip_missing if no reports for 10 minutes.
An arbitrary timeout.

Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
gossipd: adjust gossip filters if we discover we're missing gossip.
We pick up to three random peers and ask them to gossip more.

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

@rustyrussell rustyrussell force-pushed the rustyrussell:no-more-gossip-flood branch from 7afb314 to 580d172 Jun 11, 2019

@rustyrussell

This comment has been minimized.

Copy link
Contributor Author

commented Jun 11, 2019

Fix from @cdecker and trivial rebase to prevent CHANGELOG reject.

@rustyrussell rustyrussell merged commit 1e32b4a into ElementsProject:master Jun 12, 2019

1 of 2 checks passed

ackbot Need at least 1 ACKs
continuous-integration/travis-ci/pr The Travis CI build passed
Details
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
4 participants
You can’t perform that action at this time.