A recent commit modified the `IsNodeStale` method in the mocks to mirror
the actual implementation in the gossiper. As a result, we now expect
one less node announcement to be broadcast.
In this commit, we allow the gossiper to also broadcast the
corresponding node announcements, if we know of them, of a channel when
constructing its full proof. We do this to ensure peers (other than our
remote peer) receive all the relevant announcements for a channel.
The tests changes were made to ensure the new behavior introduced works
as intended. Previously, the node announcements for each test channel
announcement were not processed, so they never existed from the
gossiper's point of view.
This also addresses an existing flake in the integration test
`testNodeAnnouncement`. This problem arose due to the node announcement
being sent before the connection between Dave (node announcement sender)
and Alice (node announcement receiver) was initiated and the full
channel proof was constructed.
This commit passes the peer's quit signal to the
gossipSyncer when attempt to hand off gossip query
messages. This allows a rate-limited peer's read
handler to break out immediately, which would
otherwise remain stuck until the rate-limited
gossip syncer pulled the message.
This commit restructures the delivery of gossip
query related messages, such that they are delivered
directly to the gossip syncers. Gossip query rate
limiting was introduced in #1824 on a per-peer basis.
However, since all gossip query messages were being
delivered in the main event loop, the end result is
that one rate-limited peer could stall all other
peers.
In addition, since no other peers would be able to
submit gossip-related messages through the blocked
event loop, the back pressure would eventually rate
limit the read handlers of all peers as well.
The end result would be lengthy delays in reading
messages related to htlc forwarding.
The fix is to lift the delivery of gossip query
messages outside of the main event loop. With
this change, the rate limiting backpressure is
delivered only to the intended peer.
To mimic the current behaviour of the router's IsStaleNode, we make the
mockGraphSource consider a unknown node with no channels in the graph as
stale.
ann last
In this commit, we modify TestProcessAnnouncement to process the node
announcement last. We do this due to the recent change in the gossiper
where we'll only forward node announcements of nodes who intend to
advertised themselves within the network.
This change was needed in order to allow the node announcement to be
broadcast to the greater network, as otherwise the gossiper would assume
the node intends to stay private due to not having any advertised edges.
In this commit, we modify the gossiper to no longer broadcast
NodeAnnouncements of nodes who intend to remain private. We do this to
prevent leaking their information to the greater network.
Previously, gossiper was the only object that validated channel
updates. Because updates can also be received as part of a
failed payment session in the routing package, validation logic
needs to be available there too. Gossiper already depends on
routing and having routing call the validation logic inside
gossiper would be a circular dependency. Therefore the validation
was moved to routing.
This commit replaces the simplistic rate limiting
technique added in 557cb6e2, to use the
golang.org/x/time's rate limiter. This has the
benefit of performing traffic shaping to meet a
target maximum rate, and yet tolerate bursts. Bursts
are expected during initial sync, though should become
more rare afterwards. Performing traffic shaping with
this mechanism should improve the ability of the gossip
syncer to detect sustained bursts from the remote peer,
and penalize them appropriately.
This commit also modifies the default parameters to
accept bursts of 10 queries, with a target rate of 1
reply every 5 seconds.
This commit removes the fallback in fetchGossipSyncer
that creates a gossip syncer if one is not registered
w/in the gossiper. Now that we register gossip syncers
explicitly before reading any gossip query messages,
this should not longer be required. The fallback also
did not honor the cfg.NoChanUpdates flag, which may
have led to inconsistencies between configuration and
actual behavior.
restransmitStaleChannels
In this commit, we add an additional error check for
ErrNoGraphEdgesFound when restransmitting stale channels during the
gossiper's startup. We do this to prevent benign log messages as we'll
log that we were unable to retransmit stale channels when we didn't have
any channels in our graph to begin with.
In this commit, we aim to resolve an issue with nodes requesting for
channel announcements when receiving a channel update for a channel
they're not aware of. This can happen if a node is not caught up with
the chain or if they receive updates for zombie channels. This would
lead to a spam issue, as if a node is not caught up with the chain,
every new update they receive is premature, causing them to manually
request the backing channel announcement. Ideally, we should be able to
detect this as a potential DoS vector and ban the node responsible, but
for now we'll simply remove this functionality.
In this commit, we select on the peer's QuitSignal to allow the caller
to unblock if the peer itself is disconnecting. With this change, we now
ensure that it isn't possible for a peer to block on this method and
prevent a graceful exit.
Previosuly we would immediately return nil on the error channel for
premature ChannelUpdates, which would break the expection that a a
returned non-error meant the update was successfully added to the
database. This meant that the caller would believe the update was added
to the database, while it is actually still in volatile memory and can
be lost during restarts.
This change makes us handle premature ChannelUpdates as we handle other
premature announcements within the gossiper, by deferring sending on the
error channel until we have reprocessed the update.
Previously we wouldn't return anything in the case where the
announcement were meant for a chain we didn't recognize. After this
change we should return an error on the error channel in all flows
within the gossiper.
