discovery: fix silent remote orphan announcement proof rejection

In case of the situation when we receive remote announcement with
channel id which pointing out to unknown channel the announcement have
been silently rejected. Now such announcement will be added to the
waiting proof map.

Such solution has a serious drawback - by adding the announcement proof
without information about channel itself (that this announcement have
been received from the node eligible to sending it), we allow
overwriting the waiting proof map by `Eve` node.

discovery/service.go

@@ -226,7 +226,7 @@ func (d *AuthenticatedGossiper) ProcessRemoteAnnouncement(msg lnwire.Message,
 	select {
 	case d.networkMsgs <- nMsg:
 	case <-d.quit:
-		nMsg.err <- errors.New("discovery has shut down")
+		nMsg.err <- errors.New("gossiper has shut down")
 	}
 
 	return nMsg.err
@@ -252,7 +252,7 @@ func (d *AuthenticatedGossiper) ProcessLocalAnnouncement(msg lnwire.Message,
 	select {
 	case d.networkMsgs <- nMsg:
 	case <-d.quit:
-		nMsg.err <- errors.New("discovery has shut down")
+		nMsg.err <- errors.New("gossiper has shut down")
 	}
 
 	return nMsg.err
@@ -589,6 +589,7 @@ func (d *AuthenticatedGossiper) processNetworkAnnouncement(nMsg *networkMsg) []l
 			err := errors.Errorf("unable to validate "+
 				"channel update short_chan_id=%v: %v",
 				shortChanID, err)
+			log.Error(err)
 			nMsg.err <- err
 			return nil
 		}
@@ -686,10 +687,15 @@ func (d *AuthenticatedGossiper) processNetworkAnnouncement(nMsg *networkMsg) []l
 		// before proceeding further.
 		chanInfo, e1, e2, err := d.cfg.Router.GetChannelByID(msg.ShortChannelID)
 		if err != nil {
-			err := errors.Errorf("unable to process channel "+
-				"%v proof with short_chan_id=%v: %v", prefix,
-				shortChanID, err)
-			nMsg.err <- err
+			// TODO(andrew.shvv) this is dangerous because remote
+			// node might rewrite the waiting proof.
+			key := newProofKey(shortChanID, nMsg.isRemote)
+			d.waitingProofs[key] = msg
+
+			log.Infof("Orphan %v proof announcement with "+
+				"short_chan_id=%v, adding"+
+				"to waiting batch", prefix, shortChanID)
+			nMsg.err <- nil
 			return nil
 		}
 
@@ -799,6 +805,7 @@ func (d *AuthenticatedGossiper) processNetworkAnnouncement(nMsg *networkMsg) []l
 			nMsg.err <- err
 			return nil
 		}
+		delete(d.waitingProofs, oppositeKey)
 
 		// Proof was successfully created and now can announce the
 		// channel to the remain network.

discovery/service_test.go

@@ -51,7 +51,7 @@ var (
 	nodeKeyPriv2, _ = btcec.NewPrivateKey(btcec.S256())
 	nodeKeyPub2     = nodeKeyPriv2.PubKey()
 
-	trickleDelay     = time.Millisecond * 300
+	trickleDelay     = time.Millisecond * 100
 	proofMatureDelta uint32
 )
 
@@ -577,10 +577,10 @@ func TestPrematureAnnouncement(t *testing.T) {
 	}
 }
 
-// TestSignatureAnnouncement ensures that the AuthenticatedGossiper properly
+// TestSignatureAnnouncementLocalFirst ensures that the AuthenticatedGossiper properly
 // processes partial and fully announcement signatures message.
-func TestSignatureAnnouncement(t *testing.T) {
-	ctx, cleanup, err := createTestCtx(proofMatureDelta)
+func TestSignatureAnnouncementLocalFirst(t *testing.T) {
+	ctx, cleanup, err := createTestCtx(uint32(proofMatureDelta))
 	if err != nil {
 		t.Fatalf("can't create context: %v", err)
 	}
@@ -656,3 +656,86 @@ func TestSignatureAnnouncement(t *testing.T) {
 		}
 	}
 }
+
+// TestOrphanSignatureAnnouncement ensures that the gossiper properly
+// processes announcement with unknown channel ids.
+func TestOrphanSignatureAnnouncement(t *testing.T) {
+	ctx, cleanup, err := createTestCtx(uint32(proofMatureDelta))
+	if err != nil {
+		t.Fatalf("can't create context: %v", err)
+	}
+	defer cleanup()
+
+	batch, err := createAnnouncements(0)
+	if err != nil {
+		t.Fatalf("can't generate announcements: %v", err)
+	}
+
+	localKey := batch.nodeAnn1.NodeID
+	remoteKey := batch.nodeAnn2.NodeID
+
+	// Pretending that we receive local channel announcement from funding
+	// manager, thereby kick off the announcement exchange process, in
+	// this case the announcement should be added in the orphan batch
+	// because we haven't announce the channel yet.
+	err = <-ctx.discovery.ProcessRemoteAnnouncement(batch.remoteProofAnn, remoteKey)
+	if err != nil {
+		t.Fatalf("unable to proceed announcement: %v", err)
+	}
+
+	if len(ctx.discovery.waitingProofs) != 1 {
+		t.Fatal("local proof announcement should be stored")
+	}
+
+	// Recreate lightning network topology. Initialize router with channel
+	// between two nodes.
+	err = <-ctx.discovery.ProcessLocalAnnouncement(batch.localChanAnn, localKey)
+	if err != nil {
+		t.Fatalf("unable to process :%v", err)
+	}
+
+	select {
+	case <-ctx.broadcastedMessage:
+		t.Fatal("channel announcement was broadcast")
+	case <-time.After(2 * trickleDelay):
+	}
+
+	err = <-ctx.discovery.ProcessLocalAnnouncement(batch.chanUpdAnn, localKey)
+	if err != nil {
+		t.Fatalf("unable to process :%v", err)
+	}
+	select {
+	case <-ctx.broadcastedMessage:
+		t.Fatal("channel update announcement was broadcast")
+	case <-time.After(2 * trickleDelay):
+	}
+
+	err = <-ctx.discovery.ProcessRemoteAnnouncement(batch.chanUpdAnn, remoteKey)
+	if err != nil {
+		t.Fatalf("unable to process :%v", err)
+	}
+	select {
+	case <-ctx.broadcastedMessage:
+		t.Fatal("channel update announcement was broadcast")
+	case <-time.After(2 * trickleDelay):
+	}
+
+	// After that we process local announcement, and waiting to receive
+	// the channel announcement.
+	err = <-ctx.discovery.ProcessLocalAnnouncement(batch.localProofAnn, localKey)
+	if err != nil {
+		t.Fatalf("unable to process :%v", err)
+	}
+
+	for i := 0; i < 3; i++ {
+		select {
+		case <-ctx.broadcastedMessage:
+		case <-time.After(time.Second):
+			t.Fatal("announcement wasn't broadcast")
+		}
+	}
+
+	if len(ctx.discovery.waitingProofs) != 0 {
+		t.Fatal("index should be removed")
+	}
+}