breacharbiter: fix flaky race condition in test

This fixes a possible race condition during TestBreachSpends that could
cause the test to fail in a flaky way.

The error returned in PublishTransaction (publErr) could be modified by
the main test goroutine before PublishTransaction had a chance to
return, causing the wrong error value to be returned.

This was mostly visible as a flake during TestBreachSpends/all_spends
where adding a one second delay in the old code between the send in
publTx and the call to publMtx.Lock() would cause the last iteration of
the test loop to fail.

This is fixed by moving the lock and locally storing the expected error
value to before the send so that each call to PublishTransaction is
guaranteed to return the correct error value.

breacharbiter_test.go

@@ -1372,11 +1372,12 @@ func testBreachSpends(t *testing.T, test breachTest) {
 	// Make PublishTransaction always return ErrDoubleSpend to begin with.
 	publErr = lnwallet.ErrDoubleSpend
 	brar.cfg.PublishTransaction = func(tx *wire.MsgTx, _ string) error {
+		publMtx.Lock()
+		err := publErr
+		publMtx.Unlock()
 		publTx <- tx
 
-		publMtx.Lock()
+		return err
-		defer publMtx.Unlock()
-		return publErr
 	}
 
 	// Notify the breach arbiter about the breach.