From de961af5c6c59b9a4418391a60c79865b9ba3ed0 Mon Sep 17 00:00:00 2001 From: Elle Mouton Date: Mon, 6 Mar 2023 13:10:43 +0200 Subject: [PATCH 1/3] lntest/wait: handle the case where the pred func hangs This commit fixes a bug in the wait.NoError function. If the predicate function, f, passed to the NoError function would hang for the full timeout, then the `predErr` would remain nil and so a nil error would be returned from the function. This commit handles that case. --- lntest/wait/wait.go | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/lntest/wait/wait.go b/lntest/wait/wait.go index b6274086c..c0cd2ded2 100644 --- a/lntest/wait/wait.go +++ b/lntest/wait/wait.go @@ -58,6 +58,13 @@ func NoError(f func() error, timeout time.Duration) error { // If f() doesn't succeed within the timeout, return the last // encountered error. if err := Predicate(pred, timeout); err != nil { + // Handle the case where the passed in method, f, hangs for the + // full timeout + if predErr == nil { + return fmt.Errorf("method did not return within the " + + "timeout") + } + return predErr } From 641ae28989137fc3897f02876847732bebaa7604 Mon Sep 17 00:00:00 2001 From: Elle Mouton Date: Mon, 6 Mar 2023 13:13:51 +0200 Subject: [PATCH 2/3] chainntnfs: fix sub test names Some subtest names were incorrectly labeled by if the txindex was enabled or not but are meant to be labeled by if rpc polling is enabled or not. --- chainntnfs/bitcoindnotify/bitcoind_test.go | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/chainntnfs/bitcoindnotify/bitcoind_test.go b/chainntnfs/bitcoindnotify/bitcoind_test.go index 9a89d3454..124872de0 100644 --- a/chainntnfs/bitcoindnotify/bitcoind_test.go +++ b/chainntnfs/bitcoindnotify/bitcoind_test.go @@ -105,12 +105,12 @@ func syncNotifierWithMiner(t *testing.T, notifier *BitcoindNotifier, // TestHistoricalConfDetailsTxIndex ensures that we correctly retrieve // historical confirmation details using the backend node's txindex. func TestHistoricalConfDetailsTxIndex(t *testing.T) { - t.Run("txindex enabled", func(st *testing.T) { + t.Run("rpc polling enabled", func(st *testing.T) { st.Parallel() testHistoricalConfDetailsTxIndex(st, true) }) - t.Run("txindex disabled", func(st *testing.T) { + t.Run("rpc polling disabled", func(st *testing.T) { st.Parallel() testHistoricalConfDetailsTxIndex(st, false) }) @@ -200,12 +200,12 @@ func testHistoricalConfDetailsTxIndex(t *testing.T, rpcPolling bool) { // historical confirmation details using the set of fallback methods when the // backend node's txindex is disabled. func TestHistoricalConfDetailsNoTxIndex(t *testing.T) { - t.Run("txindex enabled", func(st *testing.T) { + t.Run("rpc polling enabled", func(st *testing.T) { st.Parallel() testHistoricalConfDetailsNoTxIndex(st, true) }) - t.Run("txindex disabled", func(st *testing.T) { + t.Run("rpc polling disabled", func(st *testing.T) { st.Parallel() testHistoricalConfDetailsNoTxIndex(st, false) }) From 7bf4eee68537efc3d52b4499020e5ff9604f5812 Mon Sep 17 00:00:00 2001 From: Elle Mouton Date: Mon, 6 Mar 2023 15:32:53 +0200 Subject: [PATCH 3/3] chainntnfs: fix temp dir creation and rpcpolling switch In the NewBitcoindBackend test util function, ensure that the `rpcpolling` param is switched on in the correct order. Also switch back to using `ioutil.TempDir` as it seems that the zmq conn strings created from `t.TempDir()` result in an invalid conn string. --- chainntnfs/test_utils.go | 19 ++++++++++++------- 1 file changed, 12 insertions(+), 7 deletions(-) diff --git a/chainntnfs/test_utils.go b/chainntnfs/test_utils.go index 401e2f052..fa6b7cd56 100644 --- a/chainntnfs/test_utils.go +++ b/chainntnfs/test_utils.go @@ -6,6 +6,7 @@ package chainntnfs import ( "errors" "fmt" + "io/ioutil" "math/rand" "os/exec" "path/filepath" @@ -197,7 +198,11 @@ func NewBitcoindBackend(t *testing.T, minerAddr string, txindex, t.Helper() - tempBitcoindDir := t.TempDir() + // We use ioutil.TempDir here instead of t.TempDir because some versions + // of bitcoind complain about the zmq connection string formats when the + // t.TempDir directory string is used. + tempBitcoindDir, err := ioutil.TempDir("", "bitcoind") + require.NoError(t, err, "unable to create temp dir") rpcPort := rand.Intn(65536-1024) + 1024 zmqBlockHost := "ipc:///" + tempBitcoindDir + "/blocks.socket" @@ -241,20 +246,20 @@ func NewBitcoindBackend(t *testing.T, minerAddr string, txindex, } if rpcpolling { + cfg.PollingConfig = &chain.PollingConfig{ + BlockPollingInterval: time.Millisecond * 20, + TxPollingInterval: time.Millisecond * 20, + } + } else { cfg.ZMQConfig = &chain.ZMQConfig{ ZMQBlockHost: zmqBlockHost, ZMQTxHost: zmqTxHost, ZMQReadDeadline: 5 * time.Second, } - } else { - cfg.PollingConfig = &chain.PollingConfig{ - BlockPollingInterval: time.Millisecond * 20, - TxPollingInterval: time.Millisecond * 20, - } } var conn *chain.BitcoindConn - err := wait.NoError(func() error { + err = wait.NoError(func() error { var err error conn, err = chain.NewBitcoindConn(cfg) if err != nil {