From 07c0277f3b91df6a30b291b2f258f56b6534c79b Mon Sep 17 00:00:00 2001 From: "Johan T. Halseth" Date: Thu, 20 May 2021 09:08:03 +0200 Subject: [PATCH 1/3] breacharbiter_test: select on quit chan on publication Since publication would deadlock on publishing the tx in case the test had failed we also select on the brar quit channel. --- breacharbiter_test.go | 13 +++++++++++-- 1 file changed, 11 insertions(+), 2 deletions(-) diff --git a/breacharbiter_test.go b/breacharbiter_test.go index 358a826a5..68de96619 100644 --- a/breacharbiter_test.go +++ b/breacharbiter_test.go @@ -1604,7 +1604,12 @@ func testBreachSpends(t *testing.T, test breachTest) { publMtx.Lock() err := publErr publMtx.Unlock() - publTx <- tx + + select { + case publTx <- tx: + case <-brar.quit: + return fmt.Errorf("brar quit") + } return err } @@ -1817,7 +1822,11 @@ func TestBreachDelayedJusticeConfirmation(t *testing.T) { // Make PublishTransaction always return succeed. brar.cfg.PublishTransaction = func(tx *wire.MsgTx, _ string) error { - publTx <- tx + select { + case publTx <- tx: + case <-brar.quit: + return fmt.Errorf("brar quit") + } return nil } From b74c1ca822a44f894e4007219e3ddc1fdf9d8a2a Mon Sep 17 00:00:00 2001 From: "Johan T. Halseth" Date: Thu, 20 May 2021 09:50:12 +0200 Subject: [PATCH 2/3] breach tests: increase cleanup timeout 1 second was not always enough for the breacharbiter to cleanup, especially in case of the more complex split sweep. --- breacharbiter_test.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/breacharbiter_test.go b/breacharbiter_test.go index 68de96619..91be814b0 100644 --- a/breacharbiter_test.go +++ b/breacharbiter_test.go @@ -2089,7 +2089,7 @@ func assertBrarCleanup(t *testing.T, brar *breachArbiter, return fmt.Errorf("channel %v not closed", chanPoint) - }, time.Second) + }, 5*time.Second) if err != nil { t.Fatalf(err.Error()) } From fb99994720b26582682bb47c183b12f2d26b53bb Mon Sep 17 00:00:00 2001 From: "Johan T. Halseth" Date: Fri, 21 May 2021 09:47:11 +0200 Subject: [PATCH 3/3] breacharbiter_test: assert publication of adjusted justice tx Since the breacharbiter will publish an adjusted justice tx after detecing a spend from the breached commitment, we could get into a race where sometime it would not get the second spend before publication, resulting in a deadlock on the publTx channel. Now we instead wait for the publication of this adjusted tx before notifying the second spend. --- breacharbiter_test.go | 33 +++++++++++++++++++++++++++------ 1 file changed, 27 insertions(+), 6 deletions(-) diff --git a/breacharbiter_test.go b/breacharbiter_test.go index 91be814b0..422e6be7e 100644 --- a/breacharbiter_test.go +++ b/breacharbiter_test.go @@ -1978,13 +1978,34 @@ func TestBreachDelayedJusticeConfirmation(t *testing.T) { require.Len(t, spending, len(justiceTx.TxIn)) require.Len(t, splits, 2) - // Finally notify that they confirm, making the breach arbiter clean - // up. - for _, tx := range splits { - for _, in := range tx.TxIn { - op := &in.PreviousOutPoint - notifier.Spend(op, blockHeight+5, tx) + // Notify that the first split confirm, making the breach arbiter + // publish another TX with the remaining inputs. + for _, in := range splits[0].TxIn { + op := &in.PreviousOutPoint + notifier.Spend(op, blockHeight+5, splits[0]) + } + + select { + + // The published tx should spend the same inputs as our second split. + case tx := <-publTx: + require.Len(t, tx.TxIn, len(splits[1].TxIn)) + for i := range tx.TxIn { + require.Equal( + t, tx.TxIn[i].PreviousOutPoint, + splits[1].TxIn[i].PreviousOutPoint, + ) } + + case <-time.After(5 * time.Second): + t.Fatalf("tx not published") + } + + // Finally notify that the second split confirms, making the breach + // arbiter clean up since all inputs have been swept. + for _, in := range splits[1].TxIn { + op := &in.PreviousOutPoint + notifier.Spend(op, blockHeight+6, splits[1]) } // Assert that the channel is fully resolved.