From e713205eeae953b71e72e252572b4c46e6549bba Mon Sep 17 00:00:00 2001 From: Wilmer Paulino Date: Mon, 22 Mar 2021 15:31:16 -0700 Subject: [PATCH] discovery: add missing error channel sends in processNetworkAnnouncement Without the error channel sends, we would block the gossip message stream upon receiving a premature channel announcement. --- discovery/gossiper.go | 14 ++++++-------- discovery/gossiper_test.go | 31 ++++++------------------------- 2 files changed, 12 insertions(+), 33 deletions(-) diff --git a/discovery/gossiper.go b/discovery/gossiper.go index bb612850..750f4e24 100644 --- a/discovery/gossiper.go +++ b/discovery/gossiper.go @@ -1612,8 +1612,7 @@ func (d *AuthenticatedGossiper) processNetworkAnnouncement( } // If the advertised inclusionary block is beyond our knowledge - // of the chain tip, then we'll put the announcement in limbo - // to be fully verified once we advance forward in the chain. + // of the chain tip, then we'll ignore for it now. d.Lock() if nMsg.isRemote && isPremature(msg.ShortChannelID, 0) { log.Infof("Announcement for chan_id=(%v), is "+ @@ -1623,6 +1622,7 @@ func (d *AuthenticatedGossiper) processNetworkAnnouncement( msg.ShortChannelID.BlockHeight, d.bestHeight) d.Unlock() + nMsg.err <- nil return nil } d.Unlock() @@ -2132,16 +2132,14 @@ func (d *AuthenticatedGossiper) processNetworkAnnouncement( // By the specification, channel announcement proofs should be // sent after some number of confirmations after channel was // registered in bitcoin blockchain. Therefore, we check if the - // proof is premature. If so we'll halt processing until the - // expected announcement height. This allows us to be tolerant - // to other clients if this constraint was changed. + // proof is premature. d.Lock() if isPremature(msg.ShortChannelID, d.cfg.ProofMatureDelta) { - log.Infof("Premature proof announcement, "+ - "current block height lower than needed: %v <"+ - " %v, add announcement to reprocessing batch", + log.Infof("Premature proof announcement, current "+ + "block height lower than needed: %v < %v", d.bestHeight, needBlockHeight) d.Unlock() + nMsg.err <- nil return nil } d.Unlock() diff --git a/discovery/gossiper_test.go b/discovery/gossiper_test.go index 067296c8..f32860b9 100644 --- a/discovery/gossiper_test.go +++ b/discovery/gossiper_test.go @@ -897,9 +897,8 @@ func TestProcessAnnouncement(t *testing.T) { } } -// TestPrematureAnnouncement checks that premature announcements are -// not propagated to the router subsystem until block with according -// block height received. +// TestPrematureAnnouncement checks that premature announcements are not +// propagated to the router subsystem. func TestPrematureAnnouncement(t *testing.T) { t.Parallel() @@ -920,8 +919,8 @@ func TestPrematureAnnouncement(t *testing.T) { // Pretending that we receive the valid channel announcement from // remote side, but block height of this announcement is greater than - // highest know to us, for that reason it should be added to the - // repeat/premature batch. + // highest know to us, for that reason it should be ignored and not + // added to the router. ca, err := createRemoteChannelAnnouncement(1) if err != nil { t.Fatalf("can't create channel announcement: %v", err) @@ -929,31 +928,13 @@ func TestPrematureAnnouncement(t *testing.T) { select { case <-ctx.gossiper.ProcessRemoteAnnouncement(ca, nodePeer): - t.Fatal("announcement was proceeded") - case <-time.After(100 * time.Millisecond): + case <-time.After(time.Second): + t.Fatal("announcement was not processed") } if len(ctx.router.infos) != 0 { t.Fatal("edge was added to router") } - - // Pretending that we receive the valid channel update announcement from - // remote side, but block height of this announcement is greater than - // highest known to us, so it should be rejected. - ua, err := createUpdateAnnouncement(1, 0, remoteKeyPriv1, timestamp) - if err != nil { - t.Fatalf("can't create update announcement: %v", err) - } - - select { - case <-ctx.gossiper.ProcessRemoteAnnouncement(ua, nodePeer): - t.Fatal("announcement was proceeded") - case <-time.After(100 * time.Millisecond): - } - - if len(ctx.router.edges) != 0 { - t.Fatal("edge update was added to router") - } } // TestSignatureAnnouncementLocalFirst ensures that the AuthenticatedGossiper