Merge pull request #5133 from wpaulino/routing-validation-cancel-deps

discovery+routing: cancel dependent jobs if parent validation fails

routing/router.go

@@ -992,7 +992,8 @@ func (r *ChannelRouter) networkHandler() {
 					update.msg,
 				)
 				if err != nil {
-					if err != ErrVBarrierShuttingDown {
+					if err != ErrVBarrierShuttingDown &&
+						err != ErrParentValidationFailed {
 						log.Warnf("unexpected error "+
 							"during validation "+
 							"barrier shutdown: %v",
@@ -1010,7 +1011,11 @@ func (r *ChannelRouter) networkHandler() {
 
 				// If this message had any dependencies, then
 				// we can now signal them to continue.
-				validationBarrier.SignalDependants(update.msg)
+				allowDependents := err == nil ||
+					IsError(err, ErrIgnored, ErrOutdated)
+				validationBarrier.SignalDependants(
+					update.msg, allowDependents,
+				)
 				if err != nil {
 					return
 				}

routing/validation_barrier.go

@@ -9,10 +9,28 @@ import (
 	"github.com/lightningnetwork/lnd/routing/route"
 )
 
-// ErrVBarrierShuttingDown signals that the barrier has been requested to
-// shutdown, and that the caller should not treat the wait condition as
-// fulfilled.
-var ErrVBarrierShuttingDown = errors.New("validation barrier shutting down")
+var (
+	// ErrVBarrierShuttingDown signals that the barrier has been requested
+	// to shutdown, and that the caller should not treat the wait condition
+	// as fulfilled.
+	ErrVBarrierShuttingDown = errors.New("validation barrier shutting down")
+
+	// ErrParentValidationFailed signals that the validation of a
+	// dependent's parent failed, so the dependent must not be processed.
+	ErrParentValidationFailed = errors.New("parent validation failed")
+)
+
+// validationSignals contains two signals which allows the ValidationBarrier to
+// communicate back to the caller whether a dependent should be processed or not
+// based on whether its parent was successfully validated. Only one of these
+// signals is to be used at a time.
+type validationSignals struct {
+	// allow is the signal used to allow a dependent to be processed.
+	allow chan struct{}
+
+	// deny is the signal used to prevent a dependent from being processed.
+	deny chan struct{}
+}
 
 // ValidationBarrier is a barrier used to ensure proper validation order while
 // concurrently validating new announcements for channel edges, and the
@@ -31,19 +49,19 @@ type ValidationBarrier struct {
 	// ChannelAnnouncement like validation job going on. Once the job has
 	// been completed, the channel will be closed unblocking any
 	// dependants.
-	chanAnnFinSignal map[lnwire.ShortChannelID]chan struct{}
+	chanAnnFinSignal map[lnwire.ShortChannelID]*validationSignals
 
 	// chanEdgeDependencies tracks any channel edge updates which should
 	// wait until the completion of the ChannelAnnouncement before
 	// proceeding. This is a dependency, as we can't validate the update
 	// before we validate the announcement which creates the channel
 	// itself.
-	chanEdgeDependencies map[lnwire.ShortChannelID]chan struct{}
+	chanEdgeDependencies map[lnwire.ShortChannelID]*validationSignals
 
 	// nodeAnnDependencies tracks any pending NodeAnnouncement validation
 	// jobs which should wait until the completion of the
 	// ChannelAnnouncement before proceeding.
-	nodeAnnDependencies map[route.Vertex]chan struct{}
+	nodeAnnDependencies map[route.Vertex]*validationSignals
 
 	quit chan struct{}
 	sync.Mutex
@@ -56,9 +74,9 @@ func NewValidationBarrier(numActiveReqs int,
 	quitChan chan struct{}) *ValidationBarrier {
 
 	v := &ValidationBarrier{
-		chanAnnFinSignal:     make(map[lnwire.ShortChannelID]chan struct{}),
-		chanEdgeDependencies: make(map[lnwire.ShortChannelID]chan struct{}),
-		nodeAnnDependencies:  make(map[route.Vertex]chan struct{}),
+		chanAnnFinSignal:     make(map[lnwire.ShortChannelID]*validationSignals),
+		chanEdgeDependencies: make(map[lnwire.ShortChannelID]*validationSignals),
+		nodeAnnDependencies:  make(map[route.Vertex]*validationSignals),
 		quit:                 quitChan,
 	}
 
@@ -107,24 +125,31 @@ func (v *ValidationBarrier) InitJobDependencies(job interface{}) {
 			// validate this announcement. All dependants will
 			// point to this same channel, so they'll be unblocked
 			// at the same time.
-			annFinCond := make(chan struct{})
-			v.chanAnnFinSignal[msg.ShortChannelID] = annFinCond
-			v.chanEdgeDependencies[msg.ShortChannelID] = annFinCond
+			signals := &validationSignals{
+				allow: make(chan struct{}),
+				deny:  make(chan struct{}),
+			}
 
-			v.nodeAnnDependencies[route.Vertex(msg.NodeID1)] = annFinCond
-			v.nodeAnnDependencies[route.Vertex(msg.NodeID2)] = annFinCond
+			v.chanAnnFinSignal[msg.ShortChannelID] = signals
+			v.chanEdgeDependencies[msg.ShortChannelID] = signals
+
+			v.nodeAnnDependencies[route.Vertex(msg.NodeID1)] = signals
+			v.nodeAnnDependencies[route.Vertex(msg.NodeID2)] = signals
 		}
 	case *channeldb.ChannelEdgeInfo:
 
 		shortID := lnwire.NewShortChanIDFromInt(msg.ChannelID)
 		if _, ok := v.chanAnnFinSignal[shortID]; !ok {
-			annFinCond := make(chan struct{})
+			signals := &validationSignals{
+				allow: make(chan struct{}),
+				deny:  make(chan struct{}),
+			}
 
-			v.chanAnnFinSignal[shortID] = annFinCond
-			v.chanEdgeDependencies[shortID] = annFinCond
+			v.chanAnnFinSignal[shortID] = signals
+			v.chanEdgeDependencies[shortID] = signals
 
-			v.nodeAnnDependencies[route.Vertex(msg.NodeKey1Bytes)] = annFinCond
-			v.nodeAnnDependencies[route.Vertex(msg.NodeKey2Bytes)] = annFinCond
+			v.nodeAnnDependencies[route.Vertex(msg.NodeKey1Bytes)] = signals
+			v.nodeAnnDependencies[route.Vertex(msg.NodeKey2Bytes)] = signals
 		}
 
 	// These other types don't have any dependants, so no further
@@ -162,8 +187,8 @@ func (v *ValidationBarrier) CompleteJob() {
 func (v *ValidationBarrier) WaitForDependants(job interface{}) error {
 
 	var (
-		signal chan struct{}
-		ok     bool
+		signals *validationSignals
+		ok      bool
 	)
 
 	v.Lock()
@@ -173,15 +198,15 @@ func (v *ValidationBarrier) WaitForDependants(job interface{}) error {
 	// completion of any active ChannelAnnouncement jobs related to them.
 	case *channeldb.ChannelEdgePolicy:
 		shortID := lnwire.NewShortChanIDFromInt(msg.ChannelID)
-		signal, ok = v.chanEdgeDependencies[shortID]
+		signals, ok = v.chanEdgeDependencies[shortID]
 	case *channeldb.LightningNode:
 		vertex := route.Vertex(msg.PubKeyBytes)
-		signal, ok = v.nodeAnnDependencies[vertex]
+		signals, ok = v.nodeAnnDependencies[vertex]
 	case *lnwire.ChannelUpdate:
-		signal, ok = v.chanEdgeDependencies[msg.ShortChannelID]
+		signals, ok = v.chanEdgeDependencies[msg.ShortChannelID]
 	case *lnwire.NodeAnnouncement:
 		vertex := route.Vertex(msg.NodeID)
-		signal, ok = v.nodeAnnDependencies[vertex]
+		signals, ok = v.nodeAnnDependencies[vertex]
 
 	// Other types of jobs can be executed immediately, so we'll just
 	// return directly.
@@ -204,7 +229,9 @@ func (v *ValidationBarrier) WaitForDependants(job interface{}) error {
 		select {
 		case <-v.quit:
 			return ErrVBarrierShuttingDown
-		case <-signal:
+		case <-signals.deny:
+			return ErrParentValidationFailed
+		case <-signals.allow:
 			return nil
 		}
 	}
@@ -212,10 +239,10 @@ func (v *ValidationBarrier) WaitForDependants(job interface{}) error {
 	return nil
 }
 
-// SignalDependants will signal any jobs that are dependent on this job that
+// SignalDependants will allow/deny any jobs that are dependent on this job that
 // they can continue execution. If the job doesn't have any dependants, then
 // this function sill exit immediately.
-func (v *ValidationBarrier) SignalDependants(job interface{}) {
+func (v *ValidationBarrier) SignalDependants(job interface{}, allow bool) {
 	v.Lock()
 	defer v.Unlock()
 
@@ -223,18 +250,26 @@ func (v *ValidationBarrier) SignalDependants(job interface{}) {
 
 	// If we've just finished executing a ChannelAnnouncement, then we'll
 	// close out the signal, and remove the signal from the map of active
-	// ones. This will allow any dependent jobs to continue execution.
+	// ones. This will allow/deny any dependent jobs to continue execution.
 	case *channeldb.ChannelEdgeInfo:
 		shortID := lnwire.NewShortChanIDFromInt(msg.ChannelID)
-		finSignal, ok := v.chanAnnFinSignal[shortID]
+		finSignals, ok := v.chanAnnFinSignal[shortID]
 		if ok {
-			close(finSignal)
+			if allow {
+				close(finSignals.allow)
+			} else {
+				close(finSignals.deny)
+			}
 			delete(v.chanAnnFinSignal, shortID)
 		}
 	case *lnwire.ChannelAnnouncement:
-		finSignal, ok := v.chanAnnFinSignal[msg.ShortChannelID]
+		finSignals, ok := v.chanAnnFinSignal[msg.ShortChannelID]
 		if ok {
-			close(finSignal)
+			if allow {
+				close(finSignals.allow)
+			} else {
+				close(finSignals.deny)
+			}
 			delete(v.chanAnnFinSignal, msg.ShortChannelID)
 		}
 

routing/validation_barrier_test.go

@@ -12,6 +12,8 @@ import (
 // TestValidationBarrierSemaphore checks basic properties of the validation
 // barrier's semaphore wrt. enqueuing/dequeuing.
 func TestValidationBarrierSemaphore(t *testing.T) {
+	t.Parallel()
+
 	const (
 		numTasks        = 8
 		numPendingTasks = 8
@@ -59,6 +61,8 @@ func TestValidationBarrierSemaphore(t *testing.T) {
 // TestValidationBarrierQuit checks that pending validation tasks will return an
 // error from WaitForDependants if the barrier's quit signal is canceled.
 func TestValidationBarrierQuit(t *testing.T) {
+	t.Parallel()
+
 	const (
 		numTasks = 8
 		timeout  = 50 * time.Millisecond
@@ -113,9 +117,14 @@ func TestValidationBarrierQuit(t *testing.T) {
 	// with the correct error.
 	for i := 0; i < numTasks; i++ {
 		switch {
-		// First half, signal completion and task semaphore
+		// Signal completion for the first half of tasks, but only allow
+		// dependents to be processed as well for the second quarter.
+		case i < numTasks/4:
+			barrier.SignalDependants(anns[i], false)
+			barrier.CompleteJob()
+
 		case i < numTasks/2:
-			barrier.SignalDependants(anns[i])
+			barrier.SignalDependants(anns[i], true)
 			barrier.CompleteJob()
 
 		// At midpoint, quit the validation barrier.
@@ -132,7 +141,10 @@ func TestValidationBarrierQuit(t *testing.T) {
 
 		switch {
 		// First half should return without failure.
-		case i < numTasks/2 && err != nil:
+		case i < numTasks/4 && err != routing.ErrParentValidationFailed:
+			t.Fatalf("unexpected failure while waiting: %v", err)
+
+		case i >= numTasks/4 && i < numTasks/2 && err != nil:
 			t.Fatalf("unexpected failure while waiting: %v", err)
 
 		// Last half should return the shutdown error.