From 1e7f2c32e97a5c90aff9e5b88fbd5ae2947b5398 Mon Sep 17 00:00:00 2001 From: Olaoluwa Osuntokun Date: Thu, 3 May 2018 16:38:54 -0700 Subject: [PATCH 1/2] test: attempt to account for internal block race in htlc force close test In this commit, we modify the testMultHopRemoteForceCloseOnChainHtlcTimeout test slightly to attempt to account for a block race between the arrival of a message betwen the contract resolver and the utxo nursery. If this message arrives "late" (relative to the speed with which we mine blocks in test), then it'll be detected as such by the utxo nursery. However, since we attempt to mine a precise number of blocks, if this happens, then we'll never actually mine that extra block to trigger a broadcast of the sweep transaction. --- lnd_test.go | 17 +++++++++++++++-- 1 file changed, 15 insertions(+), 2 deletions(-) diff --git a/lnd_test.go b/lnd_test.go index f1f38035b..622aa0d32 100644 --- a/lnd_test.go +++ b/lnd_test.go @@ -7049,9 +7049,22 @@ func testMultHopRemoteForceCloseOnChainHtlcTimeout(net *lntest.NetworkHarness, // Bob's sweeping transaction should now be found in the mempool at // this point. - _, err = waitForTxInMempool(net.Miner.Node, time.Second*20) + _, err = waitForTxInMempool(net.Miner.Node, time.Second*10) if err != nil { - t.Fatalf("unable to find bob's sweeping transaction: %v", err) + // If Bob's transaction isn't yet in the mempool, then due to + // internal message passing and the low period between blocks + // being mined, it may have been detected as a late + // registration. As a result, we'll mine another block and + // repeat the check. If it doesn't go through this time, then + // we'll fail. + if _, err := net.Miner.Node.Generate(1); err != nil { + t.Fatalf("unable to generate block: %v", err) + } + _, err = waitForTxInMempool(net.Miner.Node, time.Second*10) + if err != nil { + t.Fatalf("unable to find bob's sweeping "+ + "transaction: %v", err) + } } // If we mine an additional block, then this should confirm Bob's From e54f1ea4dbe59b2e53a94774995ae1711746c2f8 Mon Sep 17 00:00:00 2001 From: Olaoluwa Osuntokun Date: Thu, 3 May 2018 19:14:31 -0700 Subject: [PATCH 2/2] test: account for block race by mining additional block in remote htlc force close test This commit is similar to a recent commit which attempts to account for internal block races by mining a second block if the initial assertion for HTLC state fails. This can happen again if by the time that the sweeping transaction is broadcast, it doesn't make it into the next block mine. --- lnd_test.go | 15 +++++++++++++-- 1 file changed, 13 insertions(+), 2 deletions(-) diff --git a/lnd_test.go b/lnd_test.go index 622aa0d32..80db45007 100644 --- a/lnd_test.go +++ b/lnd_test.go @@ -7079,9 +7079,20 @@ func testMultHopRemoteForceCloseOnChainHtlcTimeout(net *lntest.NetworkHarness, nodes = []*lntest.HarnessNode{net.Alice} err = lntest.WaitPredicate(func() bool { return assertNumActiveHtlcs(nodes, 0) - }, time.Second*15) + }, time.Second*8) if err != nil { - t.Fatalf("alice's channel still has active htlc's") + // It may be the case that due to a race, Bob's sweeping + // transaction hasn't yet been confirmed, so we'll mine another + // block to nudge it in. If after this it still Alice will has + // an HTLC, then it's actually a test failure. + if _, err := net.Miner.Node.Generate(1); err != nil { + t.Fatalf("unable to generate block: %v", err) + } + if err = lntest.WaitPredicate(func() bool { + return assertNumActiveHtlcs(nodes, 0) + }, time.Second*8); err != nil { + t.Fatalf("alice's channel still has active htlc's") + } } // Now we'll check Bob's pending channel report. Since this was Carol's