From 95634186c28f7557c53b56c861bfdbd8657024f8 Mon Sep 17 00:00:00 2001 From: Olaoluwa Osuntokun Date: Fri, 10 Jan 2020 15:27:45 +0100 Subject: [PATCH 1/6] lntest/itest: move harness to new file This in prep for a bigger move in the next commit. --- lntest/itest/lnd_test.go | 75 ------------------------------ lntest/itest/test_harness.go | 90 ++++++++++++++++++++++++++++++++++++ 2 files changed, 90 insertions(+), 75 deletions(-) create mode 100644 lntest/itest/test_harness.go diff --git a/lntest/itest/lnd_test.go b/lntest/itest/lnd_test.go index 986fb630..03334cd2 100644 --- a/lntest/itest/lnd_test.go +++ b/lntest/itest/lnd_test.go @@ -71,81 +71,6 @@ const ( noFeeLimitMsat = math.MaxInt64 ) -// harnessTest wraps a regular testing.T providing enhanced error detection -// and propagation. All error will be augmented with a full stack-trace in -// order to aid in debugging. Additionally, any panics caused by active -// test cases will also be handled and represented as fatals. -type harnessTest struct { - t *testing.T - - // testCase is populated during test execution and represents the - // current test case. - testCase *testCase - - // lndHarness is a reference to the current network harness. Will be - // nil if not yet set up. - lndHarness *lntest.NetworkHarness -} - -// newHarnessTest creates a new instance of a harnessTest from a regular -// testing.T instance. -func newHarnessTest(t *testing.T, net *lntest.NetworkHarness) *harnessTest { - return &harnessTest{t, nil, net} -} - -// Skipf calls the underlying testing.T's Skip method, causing the current test -// to be skipped. -func (h *harnessTest) Skipf(format string, args ...interface{}) { - h.t.Skipf(format, args...) -} - -// Fatalf causes the current active test case to fail with a fatal error. All -// integration tests should mark test failures solely with this method due to -// the error stack traces it produces. -func (h *harnessTest) Fatalf(format string, a ...interface{}) { - if h.lndHarness != nil { - h.lndHarness.SaveProfilesPages() - } - - stacktrace := errors.Wrap(fmt.Sprintf(format, a...), 1).ErrorStack() - - if h.testCase != nil { - h.t.Fatalf("Failed: (%v): exited with error: \n"+ - "%v", h.testCase.name, stacktrace) - } else { - h.t.Fatalf("Error outside of test: %v", stacktrace) - } -} - -// RunTestCase executes a harness test case. Any errors or panics will be -// represented as fatal. -func (h *harnessTest) RunTestCase(testCase *testCase) { - h.testCase = testCase - defer func() { - h.testCase = nil - }() - - defer func() { - if err := recover(); err != nil { - description := errors.Wrap(err, 2).ErrorStack() - h.t.Fatalf("Failed: (%v) panicked with: \n%v", - h.testCase.name, description) - } - }() - - testCase.test(h.lndHarness, h) - - return -} - -func (h *harnessTest) Logf(format string, args ...interface{}) { - h.t.Logf(format, args...) -} - -func (h *harnessTest) Log(args ...interface{}) { - h.t.Log(args...) -} - func assertTxInBlock(t *harnessTest, block *wire.MsgBlock, txid *chainhash.Hash) { for _, tx := range block.Transactions { sha := tx.TxHash() diff --git a/lntest/itest/test_harness.go b/lntest/itest/test_harness.go new file mode 100644 index 00000000..2f8ca62a --- /dev/null +++ b/lntest/itest/test_harness.go @@ -0,0 +1,90 @@ +package itest + +import ( + "fmt" + "testing" + + "github.com/go-errors/errors" + "github.com/lightningnetwork/lnd/lntest" +) + +// harnessTest wraps a regular testing.T providing enhanced error detection +// and propagation. All error will be augmented with a full stack-trace in +// order to aid in debugging. Additionally, any panics caused by active +// test cases will also be handled and represented as fatals. +type harnessTest struct { + t *testing.T + + // testCase is populated during test execution and represents the + // current test case. + testCase *testCase + + // lndHarness is a reference to the current network harness. Will be + // nil if not yet set up. + lndHarness *lntest.NetworkHarness +} + +// newHarnessTest creates a new instance of a harnessTest from a regular +// testing.T instance. +func newHarnessTest(t *testing.T, net *lntest.NetworkHarness) *harnessTest { + return &harnessTest{t, nil, net} +} + +// Skipf calls the underlying testing.T's Skip method, causing the current test +// to be skipped. +func (h *harnessTest) Skipf(format string, args ...interface{}) { + h.t.Skipf(format, args...) +} + +// Fatalf causes the current active test case to fail with a fatal error. All +// integration tests should mark test failures solely with this method due to +// the error stack traces it produces. +func (h *harnessTest) Fatalf(format string, a ...interface{}) { + if h.lndHarness != nil { + h.lndHarness.SaveProfilesPages() + } + + stacktrace := errors.Wrap(fmt.Sprintf(format, a...), 1).ErrorStack() + + if h.testCase != nil { + h.t.Fatalf("Failed: (%v): exited with error: \n"+ + "%v", h.testCase.name, stacktrace) + } else { + h.t.Fatalf("Error outside of test: %v", stacktrace) + } +} + +// RunTestCase executes a harness test case. Any errors or panics will be +// represented as fatal. +func (h *harnessTest) RunTestCase(testCase *testCase) { + + h.testCase = testCase + defer func() { + h.testCase = nil + }() + + defer func() { + if err := recover(); err != nil { + description := errors.Wrap(err, 2).ErrorStack() + h.t.Fatalf("Failed: (%v) panicked with: \n%v", + h.testCase.name, description) + } + }() + + testCase.test(h.lndHarness, h) + + return +} + +func (h *harnessTest) Logf(format string, args ...interface{}) { + h.t.Logf(format, args...) +} + +func (h *harnessTest) Log(args ...interface{}) { + h.t.Log(args...) +} + +type testCase struct { + name string + test func(net *lntest.NetworkHarness, t *harnessTest) +} From acd615aca4c733a6ab8dc9b213e3fb680ff5f147 Mon Sep 17 00:00:00 2001 From: Olaoluwa Osuntokun Date: Fri, 10 Jan 2020 15:27:46 +0100 Subject: [PATCH 2/6] lntest: default to btcd as default test harness backend Otherwise, if we remove the build tags, then there's no default backend, and compilation will fail. --- lntest/btcd.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lntest/btcd.go b/lntest/btcd.go index 2322d8ee..43713d9f 100644 --- a/lntest/btcd.go +++ b/lntest/btcd.go @@ -1,4 +1,4 @@ -// +build btcd +// +build !bitcoind,!neutrino package lntest From c769247198c19bc2c9fb0995c2cfd0565784d79a Mon Sep 17 00:00:00 2001 From: Olaoluwa Osuntokun Date: Fri, 10 Jan 2020 15:27:48 +0100 Subject: [PATCH 3/6] lntest: allow the main test files to be buildable w/o the rpctest build tag In this commit, we modify our build tag set up to allow the main test files to be buildable w/o the current rpctest tag. We do this so that those of us that use extensions which will compile live files like vim-go can once again fix compile errors as we go in our editors. In order to do this, we now make an external `testsCases` variable, and have two variants: one that's empty (no build tag), and one that's fully populated with all our tests (build tag active). As a result, the main file will now always build regardless of if the build tag is active or not, but we'll only actually execute tests if the `testCases` variable has been populated. As sample run w/ the tag off: ``` === RUN TestLightningNetworkDaemon --- PASS: TestLightningNetworkDaemon (0.00s) PASS ok github.com/lightningnetwork/lnd/lntest/itest 0.051s ``` --- lntest/itest/lnd_channel_backup_test.go | 2 - lntest/itest/lnd_forward_interceptor_test.go | 2 - lntest/itest/lnd_macaroons_test.go | 2 - lntest/itest/lnd_mpp_test.go | 2 - .../lnd_multi-hop-error-propagation_test.go | 2 - lntest/itest/lnd_multi-hop-payments_test.go | 2 - ...d_multi-hop_htlc_local_chain_claim_test.go | 2 - .../lnd_multi-hop_htlc_local_timeout_test.go | 2 - ...ulti-hop_htlc_receiver_chain_claim_test.go | 2 - ..._multi-hop_htlc_remote_chain_claim_test.go | 2 - ..._force_close_on_chain_htlc_timeout_test.go | 2 - ..._force_close_on_chain_htlc_timeout_test.go | 2 - lntest/itest/lnd_multi-hop_test.go | 2 - .../itest/{network.go => lnd_network_test.go} | 2 - lntest/itest/lnd_onchain_test.go | 2 - lntest/itest/lnd_psbt_test.go | 2 - lntest/itest/lnd_rest_api_test.go | 2 - .../itest/lnd_send_multi_path_payment_test.go | 2 - .../itest/{signer.go => lnd_signer_test.go} | 2 - lntest/itest/lnd_single_hop_invoice_test.go | 2 - lntest/itest/lnd_test.go | 448 +----------------- lntest/itest/lnd_test_list_off_test.go | 5 + lntest/itest/lnd_test_list_on_test.go | 285 +++++++++++ lntest/itest/lnd_wumbo_channels_test.go | 2 - lntest/itest/test_harness.go | 161 +++++++ 25 files changed, 456 insertions(+), 485 deletions(-) rename lntest/itest/{network.go => lnd_network_test.go} (99%) rename lntest/itest/{signer.go => lnd_signer_test.go} (99%) create mode 100644 lntest/itest/lnd_test_list_off_test.go create mode 100644 lntest/itest/lnd_test_list_on_test.go diff --git a/lntest/itest/lnd_channel_backup_test.go b/lntest/itest/lnd_channel_backup_test.go index a55228cc..4a394c2d 100644 --- a/lntest/itest/lnd_channel_backup_test.go +++ b/lntest/itest/lnd_channel_backup_test.go @@ -1,5 +1,3 @@ -// +build rpctest - package itest import ( diff --git a/lntest/itest/lnd_forward_interceptor_test.go b/lntest/itest/lnd_forward_interceptor_test.go index 0a72d477..ed36f879 100644 --- a/lntest/itest/lnd_forward_interceptor_test.go +++ b/lntest/itest/lnd_forward_interceptor_test.go @@ -1,5 +1,3 @@ -// +build rpctest - package itest import ( diff --git a/lntest/itest/lnd_macaroons_test.go b/lntest/itest/lnd_macaroons_test.go index 8b939362..8dee9572 100644 --- a/lntest/itest/lnd_macaroons_test.go +++ b/lntest/itest/lnd_macaroons_test.go @@ -1,5 +1,3 @@ -// +build rpctest - package itest import ( diff --git a/lntest/itest/lnd_mpp_test.go b/lntest/itest/lnd_mpp_test.go index 7a30a378..c240a2b0 100644 --- a/lntest/itest/lnd_mpp_test.go +++ b/lntest/itest/lnd_mpp_test.go @@ -1,5 +1,3 @@ -// +build rpctest - package itest import ( diff --git a/lntest/itest/lnd_multi-hop-error-propagation_test.go b/lntest/itest/lnd_multi-hop-error-propagation_test.go index 244ec205..e631dd92 100644 --- a/lntest/itest/lnd_multi-hop-error-propagation_test.go +++ b/lntest/itest/lnd_multi-hop-error-propagation_test.go @@ -1,5 +1,3 @@ -// +build rpctest - package itest import ( diff --git a/lntest/itest/lnd_multi-hop-payments_test.go b/lntest/itest/lnd_multi-hop-payments_test.go index cba96883..5c546a7c 100644 --- a/lntest/itest/lnd_multi-hop-payments_test.go +++ b/lntest/itest/lnd_multi-hop-payments_test.go @@ -1,5 +1,3 @@ -// +build rpctest - package itest import ( diff --git a/lntest/itest/lnd_multi-hop_htlc_local_chain_claim_test.go b/lntest/itest/lnd_multi-hop_htlc_local_chain_claim_test.go index 256a8afe..fcb3d0d0 100644 --- a/lntest/itest/lnd_multi-hop_htlc_local_chain_claim_test.go +++ b/lntest/itest/lnd_multi-hop_htlc_local_chain_claim_test.go @@ -1,5 +1,3 @@ -// +build rpctest - package itest import ( diff --git a/lntest/itest/lnd_multi-hop_htlc_local_timeout_test.go b/lntest/itest/lnd_multi-hop_htlc_local_timeout_test.go index 67b13b56..6e61b458 100644 --- a/lntest/itest/lnd_multi-hop_htlc_local_timeout_test.go +++ b/lntest/itest/lnd_multi-hop_htlc_local_timeout_test.go @@ -1,5 +1,3 @@ -// +build rpctest - package itest import ( diff --git a/lntest/itest/lnd_multi-hop_htlc_receiver_chain_claim_test.go b/lntest/itest/lnd_multi-hop_htlc_receiver_chain_claim_test.go index a19e51d1..fb118940 100644 --- a/lntest/itest/lnd_multi-hop_htlc_receiver_chain_claim_test.go +++ b/lntest/itest/lnd_multi-hop_htlc_receiver_chain_claim_test.go @@ -1,5 +1,3 @@ -// +build rpctest - package itest import ( diff --git a/lntest/itest/lnd_multi-hop_htlc_remote_chain_claim_test.go b/lntest/itest/lnd_multi-hop_htlc_remote_chain_claim_test.go index 7e3fe616..574ecf71 100644 --- a/lntest/itest/lnd_multi-hop_htlc_remote_chain_claim_test.go +++ b/lntest/itest/lnd_multi-hop_htlc_remote_chain_claim_test.go @@ -1,5 +1,3 @@ -// +build rpctest - package itest import ( diff --git a/lntest/itest/lnd_multi-hop_local_force_close_on_chain_htlc_timeout_test.go b/lntest/itest/lnd_multi-hop_local_force_close_on_chain_htlc_timeout_test.go index a2a97269..449c4573 100644 --- a/lntest/itest/lnd_multi-hop_local_force_close_on_chain_htlc_timeout_test.go +++ b/lntest/itest/lnd_multi-hop_local_force_close_on_chain_htlc_timeout_test.go @@ -1,5 +1,3 @@ -// +build rpctest - package itest import ( diff --git a/lntest/itest/lnd_multi-hop_remote_force_close_on_chain_htlc_timeout_test.go b/lntest/itest/lnd_multi-hop_remote_force_close_on_chain_htlc_timeout_test.go index 90fc24eb..53e56227 100644 --- a/lntest/itest/lnd_multi-hop_remote_force_close_on_chain_htlc_timeout_test.go +++ b/lntest/itest/lnd_multi-hop_remote_force_close_on_chain_htlc_timeout_test.go @@ -1,5 +1,3 @@ -// +build rpctest - package itest import ( diff --git a/lntest/itest/lnd_multi-hop_test.go b/lntest/itest/lnd_multi-hop_test.go index 100a5842..29817461 100644 --- a/lntest/itest/lnd_multi-hop_test.go +++ b/lntest/itest/lnd_multi-hop_test.go @@ -1,5 +1,3 @@ -// +build rpctest - package itest import ( diff --git a/lntest/itest/network.go b/lntest/itest/lnd_network_test.go similarity index 99% rename from lntest/itest/network.go rename to lntest/itest/lnd_network_test.go index c934c1ab..8ce168b0 100644 --- a/lntest/itest/network.go +++ b/lntest/itest/lnd_network_test.go @@ -1,5 +1,3 @@ -// +build rpctest - package itest import ( diff --git a/lntest/itest/lnd_onchain_test.go b/lntest/itest/lnd_onchain_test.go index 7cc4df11..bb03165d 100644 --- a/lntest/itest/lnd_onchain_test.go +++ b/lntest/itest/lnd_onchain_test.go @@ -1,5 +1,3 @@ -// +build rpctest - package itest import ( diff --git a/lntest/itest/lnd_psbt_test.go b/lntest/itest/lnd_psbt_test.go index 04617dc5..b2affc03 100644 --- a/lntest/itest/lnd_psbt_test.go +++ b/lntest/itest/lnd_psbt_test.go @@ -1,5 +1,3 @@ -// +build rpctest - package itest import ( diff --git a/lntest/itest/lnd_rest_api_test.go b/lntest/itest/lnd_rest_api_test.go index c296e3a0..f0b402d8 100644 --- a/lntest/itest/lnd_rest_api_test.go +++ b/lntest/itest/lnd_rest_api_test.go @@ -1,5 +1,3 @@ -// +build rpctest - package itest import ( diff --git a/lntest/itest/lnd_send_multi_path_payment_test.go b/lntest/itest/lnd_send_multi_path_payment_test.go index 453f1676..4065cd0d 100644 --- a/lntest/itest/lnd_send_multi_path_payment_test.go +++ b/lntest/itest/lnd_send_multi_path_payment_test.go @@ -1,5 +1,3 @@ -// +build rpctest - package itest import ( diff --git a/lntest/itest/signer.go b/lntest/itest/lnd_signer_test.go similarity index 99% rename from lntest/itest/signer.go rename to lntest/itest/lnd_signer_test.go index 47e4fd8b..02402145 100644 --- a/lntest/itest/signer.go +++ b/lntest/itest/lnd_signer_test.go @@ -1,5 +1,3 @@ -// +build rpctest - package itest import ( diff --git a/lntest/itest/lnd_single_hop_invoice_test.go b/lntest/itest/lnd_single_hop_invoice_test.go index 6f05841b..58c26d0b 100644 --- a/lntest/itest/lnd_single_hop_invoice_test.go +++ b/lntest/itest/lnd_single_hop_invoice_test.go @@ -1,5 +1,3 @@ -// +build rpctest - package itest import ( diff --git a/lntest/itest/lnd_test.go b/lntest/itest/lnd_test.go index 03334cd2..49254d30 100644 --- a/lntest/itest/lnd_test.go +++ b/lntest/itest/lnd_test.go @@ -1,5 +1,3 @@ -// +build rpctest - package itest import ( @@ -24,7 +22,6 @@ import ( "github.com/btcsuite/btcd/blockchain" "github.com/btcsuite/btcd/btcjson" - "github.com/btcsuite/btcd/chaincfg" "github.com/btcsuite/btcd/chaincfg/chainhash" "github.com/btcsuite/btcd/integration/rpctest" "github.com/btcsuite/btcd/rpcclient" @@ -55,66 +52,6 @@ import ( "github.com/stretchr/testify/require" ) -var ( - harnessNetParams = &chaincfg.RegressionNetParams -) - -const ( - testFeeBase = 1e+6 - defaultCSV = lntest.DefaultCSV - defaultTimeout = lntest.DefaultTimeout - minerMempoolTimeout = lntest.MinerMempoolTimeout - channelOpenTimeout = lntest.ChannelOpenTimeout - channelCloseTimeout = lntest.ChannelCloseTimeout - itestLndBinary = "../../lnd-itest" - anchorSize = 330 - noFeeLimitMsat = math.MaxInt64 -) - -func assertTxInBlock(t *harnessTest, block *wire.MsgBlock, txid *chainhash.Hash) { - for _, tx := range block.Transactions { - sha := tx.TxHash() - if bytes.Equal(txid[:], sha[:]) { - return - } - } - - t.Fatalf("tx was not included in block") -} - -func assertWalletUnspent(t *harnessTest, node *lntest.HarnessNode, out *lnrpc.OutPoint) { - t.t.Helper() - - err := wait.NoError(func() error { - ctxt, cancel := context.WithTimeout(context.Background(), defaultTimeout) - defer cancel() - unspent, err := node.ListUnspent(ctxt, &lnrpc.ListUnspentRequest{}) - if err != nil { - return err - } - - err = errors.New("tx with wanted txhash never found") - for _, utxo := range unspent.Utxos { - if !bytes.Equal(utxo.Outpoint.TxidBytes, out.TxidBytes) { - continue - } - - err = errors.New("wanted output is not a wallet utxo") - if utxo.Outpoint.OutputIndex != out.OutputIndex { - continue - } - - return nil - } - - return err - }, defaultTimeout) - if err != nil { - t.Fatalf("outpoint %s not unspent by %s's wallet: %v", out, - node.Name(), err) - } -} - func rpcPointToWirePoint(t *harnessTest, chanPoint *lnrpc.ChannelPoint) wire.OutPoint { txid, err := lnd.GetChanPointFundingTxid(chanPoint) if err != nil { @@ -127,50 +64,6 @@ func rpcPointToWirePoint(t *harnessTest, chanPoint *lnrpc.ChannelPoint) wire.Out } } -// mineBlocks mine 'num' of blocks and check that blocks are present in -// node blockchain. numTxs should be set to the number of transactions -// (excluding the coinbase) we expect to be included in the first mined block. -func mineBlocks(t *harnessTest, net *lntest.NetworkHarness, - num uint32, numTxs int) []*wire.MsgBlock { - - // If we expect transactions to be included in the blocks we'll mine, - // we wait here until they are seen in the miner's mempool. - var txids []*chainhash.Hash - var err error - if numTxs > 0 { - txids, err = waitForNTxsInMempool( - net.Miner.Node, numTxs, minerMempoolTimeout, - ) - if err != nil { - t.Fatalf("unable to find txns in mempool: %v", err) - } - } - - blocks := make([]*wire.MsgBlock, num) - - blockHashes, err := net.Miner.Node.Generate(num) - if err != nil { - t.Fatalf("unable to generate blocks: %v", err) - } - - for i, blockHash := range blockHashes { - block, err := net.Miner.Node.GetBlock(blockHash) - if err != nil { - t.Fatalf("unable to get block: %v", err) - } - - blocks[i] = block - } - - // Finally, assert that all the transactions were included in the first - // block. - for _, txid := range txids { - assertTxInBlock(t, blocks[0], txid) - } - - return blocks -} - // openChannelStream blocks until an OpenChannel request for a channel funding // by alice succeeds. If it does, a stream client is returned to receive events // about the opening channel. @@ -774,11 +667,6 @@ func getChanInfo(ctx context.Context, node *lntest.HarnessNode) ( return channelInfo.Channels[0], nil } -const ( - AddrTypeWitnessPubkeyHash = lnrpc.AddressType_WITNESS_PUBKEY_HASH - AddrTypeNestedPubkeyHash = lnrpc.AddressType_NESTED_PUBKEY_HASH -) - // testGetRecoveryInfo checks whether lnd gives the right information about // the wallet recovery process. func testGetRecoveryInfo(net *lntest.NetworkHarness, t *harnessTest) { @@ -7460,50 +7348,6 @@ func testMaxPendingChannels(net *lntest.NetworkHarness, t *harnessTest) { } } -// waitForTxInMempool polls until finding one transaction in the provided -// miner's mempool. An error is returned if *one* transaction isn't found within -// the given timeout. -func waitForTxInMempool(miner *rpcclient.Client, - timeout time.Duration) (*chainhash.Hash, error) { - - txs, err := waitForNTxsInMempool(miner, 1, timeout) - if err != nil { - return nil, err - } - - return txs[0], err -} - -// waitForNTxsInMempool polls until finding the desired number of transactions -// in the provided miner's mempool. An error is returned if this number is not -// met after the given timeout. -func waitForNTxsInMempool(miner *rpcclient.Client, n int, - timeout time.Duration) ([]*chainhash.Hash, error) { - - breakTimeout := time.After(timeout) - ticker := time.NewTicker(50 * time.Millisecond) - defer ticker.Stop() - - var err error - var mempool []*chainhash.Hash - for { - select { - case <-breakTimeout: - return nil, fmt.Errorf("wanted %v, found %v txs "+ - "in mempool: %v", n, len(mempool), mempool) - case <-ticker.C: - mempool, err = miner.GetRawMempool() - if err != nil { - return nil, err - } - - if len(mempool) == n { - return mempool, nil - } - } - } -} - // getNTxsFromMempool polls until finding the desired number of transactions in // the provided miner's mempool and returns the full transactions to the caller. func getNTxsFromMempool(miner *rpcclient.Client, n int, @@ -14078,296 +13922,14 @@ func getPaymentResult(stream routerrpc.Router_SendPaymentV2Client) ( } } -type testCase struct { - name string - test func(net *lntest.NetworkHarness, t *harnessTest) -} - -var testsCases = []*testCase{ - { - name: "sweep coins", - test: testSweepAllCoins, - }, - { - name: "recovery info", - test: testGetRecoveryInfo, - }, - { - name: "onchain fund recovery", - test: testOnchainFundRecovery, - }, - { - name: "basic funding flow", - test: testBasicChannelFunding, - }, - { - name: "unconfirmed channel funding", - test: testUnconfirmedChannelFunding, - }, - { - name: "update channel policy", - test: testUpdateChannelPolicy, - }, - { - name: "open channel reorg test", - test: testOpenChannelAfterReorg, - }, - { - name: "disconnecting target peer", - test: testDisconnectingTargetPeer, - }, - { - name: "graph topology notifications", - test: testGraphTopologyNotifications, - }, - { - name: "funding flow persistence", - test: testChannelFundingPersistence, - }, - { - name: "channel force closure", - test: testChannelForceClosure, - }, - { - name: "channel balance", - test: testChannelBalance, - }, - { - name: "channel unsettled balance", - test: testChannelUnsettledBalance, - }, - { - name: "single hop invoice", - test: testSingleHopInvoice, - }, - { - name: "sphinx replay persistence", - test: testSphinxReplayPersistence, - }, - { - name: "list channels", - test: testListChannels, - }, - { - name: "list outgoing payments", - test: testListPayments, - }, - { - name: "max pending channel", - test: testMaxPendingChannels, - }, - { - name: "multi-hop payments", - test: testMultiHopPayments, - }, - { - name: "single-hop send to route", - test: testSingleHopSendToRoute, - }, - { - name: "multi-hop send to route", - test: testMultiHopSendToRoute, - }, - { - name: "send to route error propagation", - test: testSendToRouteErrorPropagation, - }, - { - name: "unannounced channels", - test: testUnannouncedChannels, - }, - { - name: "private channels", - test: testPrivateChannels, - }, - { - name: "invoice routing hints", - test: testInvoiceRoutingHints, - }, - { - name: "multi-hop payments over private channels", - test: testMultiHopOverPrivateChannels, - }, - { - name: "multiple channel creation and update subscription", - test: testBasicChannelCreationAndUpdates, - }, - { - name: "invoice update subscription", - test: testInvoiceSubscriptions, - }, - { - name: "multi-hop htlc error propagation", - test: testHtlcErrorPropagation, - }, - { - name: "reject onward htlc", - test: testRejectHTLC, - }, - // TODO(roasbeef): multi-path integration test - { - name: "node announcement", - test: testNodeAnnouncement, - }, - { - name: "node sign verify", - test: testNodeSignVerify, - }, - { - name: "derive shared key", - test: testDeriveSharedKey, - }, - { - name: "async payments benchmark", - test: testAsyncPayments, - }, - { - name: "async bidirectional payments", - test: testBidirectionalAsyncPayments, - }, - { - name: "test multi-hop htlc", - test: testMultiHopHtlcClaims, - }, - { - name: "switch circuit persistence", - test: testSwitchCircuitPersistence, - }, - { - name: "switch offline delivery", - test: testSwitchOfflineDelivery, - }, - { - name: "switch offline delivery persistence", - test: testSwitchOfflineDeliveryPersistence, - }, - { - name: "switch offline delivery outgoing offline", - test: testSwitchOfflineDeliveryOutgoingOffline, - }, - { - // TODO(roasbeef): test always needs to be last as Bob's state - // is borked since we trick him into attempting to cheat Alice? - name: "revoked uncooperative close retribution", - test: testRevokedCloseRetribution, - }, - { - name: "failing link", - test: testFailingChannel, - }, - { - name: "garbage collect link nodes", - test: testGarbageCollectLinkNodes, - }, - { - name: "abandonchannel", - test: testAbandonChannel, - }, - { - name: "revoked uncooperative close retribution zero value remote output", - test: testRevokedCloseRetributionZeroValueRemoteOutput, - }, - { - name: "revoked uncooperative close retribution remote hodl", - test: testRevokedCloseRetributionRemoteHodl, - }, - { - name: "revoked uncooperative close retribution altruist watchtower", - test: testRevokedCloseRetributionAltruistWatchtower, - }, - { - name: "data loss protection", - test: testDataLossProtection, - }, - { - name: "query routes", - test: testQueryRoutes, - }, - { - name: "route fee cutoff", - test: testRouteFeeCutoff, - }, - { - name: "send update disable channel", - test: testSendUpdateDisableChannel, - }, - { - name: "streaming channel backup update", - test: testChannelBackupUpdates, - }, - { - name: "export channel backup", - test: testExportChannelBackup, - }, - { - name: "channel backup restore", - test: testChannelBackupRestore, - }, - { - name: "hold invoice sender persistence", - test: testHoldInvoicePersistence, - }, - { - name: "cpfp", - test: testCPFP, - }, - { - name: "macaroon authentication", - test: testMacaroonAuthentication, - }, - { - name: "bake macaroon", - test: testBakeMacaroon, - }, - { - name: "delete macaroon id", - test: testDeleteMacaroonID, - }, - { - name: "immediate payment after channel opened", - test: testPaymentFollowingChannelOpen, - }, - { - name: "external channel funding", - test: testExternalFundingChanPoint, - }, - { - name: "psbt channel funding", - test: testPsbtChanFunding, - }, - { - name: "sendtoroute multi path payment", - test: testSendToRouteMultiPath, - }, - { - name: "send multi path payment", - test: testSendMultiPathPayment, - }, - { - name: "REST API", - test: testRestApi, - }, - { - name: "intercept forwarded htlc packets", - test: testForwardInterceptor, - }, - { - name: "wumbo channels", - test: testWumboChannels, - }, - { - name: "maximum channel size", - test: testMaxChannelSize, - }, - { - name: "connection timeout", - test: testNetworkConnectionTimeout, - }, -} - // TestLightningNetworkDaemon performs a series of integration tests amongst a // programmatically driven network of lnd nodes. func TestLightningNetworkDaemon(t *testing.T) { + // If no tests are regsitered, then we can exit early. + if len(testsCases) == 0 { + t.Skip("integration tests not selected with flag 'rpctest'") + } + ht := newHarnessTest(t, nil) // Declare the network harness here to gain access to its diff --git a/lntest/itest/lnd_test_list_off_test.go b/lntest/itest/lnd_test_list_off_test.go new file mode 100644 index 00000000..ae18d5e0 --- /dev/null +++ b/lntest/itest/lnd_test_list_off_test.go @@ -0,0 +1,5 @@ +// +build !rpctest + +package itest + +var testsCases = []*testCase{} diff --git a/lntest/itest/lnd_test_list_on_test.go b/lntest/itest/lnd_test_list_on_test.go new file mode 100644 index 00000000..e4cb916e --- /dev/null +++ b/lntest/itest/lnd_test_list_on_test.go @@ -0,0 +1,285 @@ +// +build rpctest + +package itest + +var testsCases = []*testCase{ + { + name: "sweep coins", + test: testSweepAllCoins, + }, + { + name: "recovery info", + test: testGetRecoveryInfo, + }, + { + name: "onchain fund recovery", + test: testOnchainFundRecovery, + }, + { + name: "basic funding flow", + test: testBasicChannelFunding, + }, + { + name: "unconfirmed channel funding", + test: testUnconfirmedChannelFunding, + }, + { + name: "update channel policy", + test: testUpdateChannelPolicy, + }, + { + name: "open channel reorg test", + test: testOpenChannelAfterReorg, + }, + { + name: "disconnecting target peer", + test: testDisconnectingTargetPeer, + }, + { + name: "graph topology notifications", + test: testGraphTopologyNotifications, + }, + { + name: "funding flow persistence", + test: testChannelFundingPersistence, + }, + { + name: "channel force closure", + test: testChannelForceClosure, + }, + { + name: "channel balance", + test: testChannelBalance, + }, + { + name: "channel unsettled balance", + test: testChannelUnsettledBalance, + }, + { + name: "single hop invoice", + test: testSingleHopInvoice, + }, + { + name: "sphinx replay persistence", + test: testSphinxReplayPersistence, + }, + { + name: "list channels", + test: testListChannels, + }, + { + name: "list outgoing payments", + test: testListPayments, + }, + { + name: "max pending channel", + test: testMaxPendingChannels, + }, + { + name: "multi-hop payments", + test: testMultiHopPayments, + }, + { + name: "single-hop send to route", + test: testSingleHopSendToRoute, + }, + { + name: "multi-hop send to route", + test: testMultiHopSendToRoute, + }, + { + name: "send to route error propagation", + test: testSendToRouteErrorPropagation, + }, + { + name: "unannounced channels", + test: testUnannouncedChannels, + }, + { + name: "private channels", + test: testPrivateChannels, + }, + { + name: "invoice routing hints", + test: testInvoiceRoutingHints, + }, + { + name: "multi-hop payments over private channels", + test: testMultiHopOverPrivateChannels, + }, + { + name: "multiple channel creation and update subscription", + test: testBasicChannelCreationAndUpdates, + }, + { + name: "invoice update subscription", + test: testInvoiceSubscriptions, + }, + { + name: "multi-hop htlc error propagation", + test: testHtlcErrorPropagation, + }, + { + name: "reject onward htlc", + test: testRejectHTLC, + }, + // TODO(roasbeef): multi-path integration test + { + name: "node announcement", + test: testNodeAnnouncement, + }, + { + name: "node sign verify", + test: testNodeSignVerify, + }, + { + name: "derive shared key", + test: testDeriveSharedKey, + }, + { + name: "async payments benchmark", + test: testAsyncPayments, + }, + { + name: "async bidirectional payments", + test: testBidirectionalAsyncPayments, + }, + { + name: "test multi-hop htlc", + test: testMultiHopHtlcClaims, + }, + { + name: "switch circuit persistence", + test: testSwitchCircuitPersistence, + }, + { + name: "switch offline delivery", + test: testSwitchOfflineDelivery, + }, + { + name: "switch offline delivery persistence", + test: testSwitchOfflineDeliveryPersistence, + }, + { + name: "switch offline delivery outgoing offline", + test: testSwitchOfflineDeliveryOutgoingOffline, + }, + { + // TODO(roasbeef): test always needs to be last as Bob's state + // is borked since we trick him into attempting to cheat Alice? + name: "revoked uncooperative close retribution", + test: testRevokedCloseRetribution, + }, + { + name: "failing link", + test: testFailingChannel, + }, + { + name: "garbage collect link nodes", + test: testGarbageCollectLinkNodes, + }, + { + name: "abandonchannel", + test: testAbandonChannel, + }, + { + name: "revoked uncooperative close retribution zero value remote output", + test: testRevokedCloseRetributionZeroValueRemoteOutput, + }, + { + name: "revoked uncooperative close retribution remote hodl", + test: testRevokedCloseRetributionRemoteHodl, + }, + { + name: "revoked uncooperative close retribution altruist watchtower", + test: testRevokedCloseRetributionAltruistWatchtower, + }, + { + name: "data loss protection", + test: testDataLossProtection, + }, + { + name: "query routes", + test: testQueryRoutes, + }, + { + name: "route fee cutoff", + test: testRouteFeeCutoff, + }, + { + name: "send update disable channel", + test: testSendUpdateDisableChannel, + }, + { + name: "streaming channel backup update", + test: testChannelBackupUpdates, + }, + { + name: "export channel backup", + test: testExportChannelBackup, + }, + { + name: "channel backup restore", + test: testChannelBackupRestore, + }, + { + name: "hold invoice sender persistence", + test: testHoldInvoicePersistence, + }, + { + name: "cpfp", + test: testCPFP, + }, + { + name: "macaroon authentication", + test: testMacaroonAuthentication, + }, + { + name: "bake macaroon", + test: testBakeMacaroon, + }, + { + name: "delete macaroon id", + test: testDeleteMacaroonID, + }, + { + name: "immediate payment after channel opened", + test: testPaymentFollowingChannelOpen, + }, + { + name: "external channel funding", + test: testExternalFundingChanPoint, + }, + { + name: "psbt channel funding", + test: testPsbtChanFunding, + }, + { + name: "sendtoroute multi path payment", + test: testSendToRouteMultiPath, + }, + { + name: "send multi path payment", + test: testSendMultiPathPayment, + }, + { + name: "REST API", + test: testRestApi, + }, + { + name: "intercept forwarded htlc packets", + test: testForwardInterceptor, + }, + { + name: "wumbo channels", + test: testWumboChannels, + }, + { + name: "maximum channel size", + test: testMaxChannelSize, + }, + { + name: "connection timeout", + test: testNetworkConnectionTimeout, + }, +} diff --git a/lntest/itest/lnd_wumbo_channels_test.go b/lntest/itest/lnd_wumbo_channels_test.go index 6084b596..b4f48c3b 100644 --- a/lntest/itest/lnd_wumbo_channels_test.go +++ b/lntest/itest/lnd_wumbo_channels_test.go @@ -1,5 +1,3 @@ -// +build rpctest - package itest import ( diff --git a/lntest/itest/test_harness.go b/lntest/itest/test_harness.go index 2f8ca62a..daf089c0 100644 --- a/lntest/itest/test_harness.go +++ b/lntest/itest/test_harness.go @@ -1,11 +1,40 @@ package itest import ( + "bytes" + "context" "fmt" + "math" "testing" + "time" + "github.com/btcsuite/btcd/chaincfg" + "github.com/btcsuite/btcd/chaincfg/chainhash" + "github.com/btcsuite/btcd/rpcclient" + "github.com/btcsuite/btcd/wire" "github.com/go-errors/errors" + "github.com/lightningnetwork/lnd/lnrpc" "github.com/lightningnetwork/lnd/lntest" + "github.com/lightningnetwork/lnd/lntest/wait" +) + +var ( + harnessNetParams = &chaincfg.RegressionNetParams +) + +const ( + testFeeBase = 1e+6 + defaultCSV = lntest.DefaultCSV + defaultTimeout = lntest.DefaultTimeout + minerMempoolTimeout = lntest.MinerMempoolTimeout + channelOpenTimeout = lntest.ChannelOpenTimeout + channelCloseTimeout = lntest.ChannelCloseTimeout + itestLndBinary = "../../lnd-itest" + anchorSize = 330 + noFeeLimitMsat = math.MaxInt64 + + AddrTypeWitnessPubkeyHash = lnrpc.AddressType_WITNESS_PUBKEY_HASH + AddrTypeNestedPubkeyHash = lnrpc.AddressType_NESTED_PUBKEY_HASH ) // harnessTest wraps a regular testing.T providing enhanced error detection @@ -88,3 +117,135 @@ type testCase struct { name string test func(net *lntest.NetworkHarness, t *harnessTest) } + +// waitForTxInMempool polls until finding one transaction in the provided +// miner's mempool. An error is returned if *one* transaction isn't found within +// the given timeout. +func waitForTxInMempool(miner *rpcclient.Client, + timeout time.Duration) (*chainhash.Hash, error) { + + txs, err := waitForNTxsInMempool(miner, 1, timeout) + if err != nil { + return nil, err + } + + return txs[0], err +} + +// waitForNTxsInMempool polls until finding the desired number of transactions +// in the provided miner's mempool. An error is returned if this number is not +// met after the given timeout. +func waitForNTxsInMempool(miner *rpcclient.Client, n int, + timeout time.Duration) ([]*chainhash.Hash, error) { + + breakTimeout := time.After(timeout) + ticker := time.NewTicker(50 * time.Millisecond) + defer ticker.Stop() + + var err error + var mempool []*chainhash.Hash + for { + select { + case <-breakTimeout: + return nil, fmt.Errorf("wanted %v, found %v txs "+ + "in mempool: %v", n, len(mempool), mempool) + case <-ticker.C: + mempool, err = miner.GetRawMempool() + if err != nil { + return nil, err + } + + if len(mempool) == n { + return mempool, nil + } + } + } +} + +// mineBlocks mine 'num' of blocks and check that blocks are present in +// node blockchain. numTxs should be set to the number of transactions +// (excluding the coinbase) we expect to be included in the first mined block. +func mineBlocks(t *harnessTest, net *lntest.NetworkHarness, + num uint32, numTxs int) []*wire.MsgBlock { + + // If we expect transactions to be included in the blocks we'll mine, + // we wait here until they are seen in the miner's mempool. + var txids []*chainhash.Hash + var err error + if numTxs > 0 { + txids, err = waitForNTxsInMempool( + net.Miner.Node, numTxs, minerMempoolTimeout, + ) + if err != nil { + t.Fatalf("unable to find txns in mempool: %v", err) + } + } + + blocks := make([]*wire.MsgBlock, num) + + blockHashes, err := net.Miner.Node.Generate(num) + if err != nil { + t.Fatalf("unable to generate blocks: %v", err) + } + + for i, blockHash := range blockHashes { + block, err := net.Miner.Node.GetBlock(blockHash) + if err != nil { + t.Fatalf("unable to get block: %v", err) + } + + blocks[i] = block + } + + // Finally, assert that all the transactions were included in the first + // block. + for _, txid := range txids { + assertTxInBlock(t, blocks[0], txid) + } + + return blocks +} + +func assertTxInBlock(t *harnessTest, block *wire.MsgBlock, txid *chainhash.Hash) { + for _, tx := range block.Transactions { + sha := tx.TxHash() + if bytes.Equal(txid[:], sha[:]) { + return + } + } + + t.Fatalf("tx was not included in block") +} + +func assertWalletUnspent(t *harnessTest, node *lntest.HarnessNode, out *lnrpc.OutPoint) { + t.t.Helper() + + err := wait.NoError(func() error { + ctxt, cancel := context.WithTimeout(context.Background(), defaultTimeout) + defer cancel() + unspent, err := node.ListUnspent(ctxt, &lnrpc.ListUnspentRequest{}) + if err != nil { + return err + } + + err = errors.New("tx with wanted txhash never found") + for _, utxo := range unspent.Utxos { + if !bytes.Equal(utxo.Outpoint.TxidBytes, out.TxidBytes) { + continue + } + + err = errors.New("wanted output is not a wallet utxo") + if utxo.Outpoint.OutputIndex != out.OutputIndex { + continue + } + + return nil + } + + return err + }, defaultTimeout) + if err != nil { + t.Fatalf("outpoint %s not unspent by %s's wallet: %v", out, + node.Name(), err) + } +} From 719e32830dcc8f6161d6fda50d1401ecbd46f7c6 Mon Sep 17 00:00:00 2001 From: Olaoluwa Osuntokun Date: Fri, 10 Jan 2020 15:27:50 +0100 Subject: [PATCH 4/6] lntest: fix most linter warnings, silence rest We fix all linter issues except for the 'lostcontext' and 'unparam' ones as those are too numerous and would increase the diff even more. Therefore we silence them in the itest directory for now. Because the linter is still not build tag aware, we also have to silence the unused and deadcode sub linters to not get false positives. --- .golangci.yml | 8 +++ lntest/harness.go | 1 - lntest/itest/lnd_channel_backup_test.go | 3 +- lntest/itest/lnd_forward_interceptor_test.go | 4 +- lntest/itest/lnd_macaroons_test.go | 4 +- lntest/itest/lnd_mpp_test.go | 2 +- lntest/itest/lnd_multi-hop-payments_test.go | 2 +- .../lnd_multi-hop_htlc_local_timeout_test.go | 17 +---- ...ulti-hop_htlc_receiver_chain_claim_test.go | 4 +- ..._force_close_on_chain_htlc_timeout_test.go | 11 +--- ..._force_close_on_chain_htlc_timeout_test.go | 11 +--- lntest/itest/lnd_multi-hop_test.go | 1 + lntest/itest/lnd_network_test.go | 4 +- lntest/itest/lnd_psbt_test.go | 2 +- lntest/itest/lnd_rest_api_test.go | 8 ++- lntest/itest/lnd_single_hop_invoice_test.go | 1 - lntest/itest/lnd_test.go | 64 +++++++------------ lntest/itest/lnd_test_list_on_test.go | 2 +- lntest/itest/test_harness.go | 2 - 19 files changed, 58 insertions(+), 93 deletions(-) diff --git a/.golangci.yml b/.golangci.yml index c0057fce..1ffef44b 100644 --- a/.golangci.yml +++ b/.golangci.yml @@ -65,3 +65,11 @@ issues: - path: _test\.go linters: - gosec + + # Fix false positives because of build flags in itest directory. + - path: lntest/itest/.* + linters: + - unused + - deadcode + - unparam + - govet diff --git a/lntest/harness.go b/lntest/harness.go index f8c0b7fc..aa56af72 100644 --- a/lntest/harness.go +++ b/lntest/harness.go @@ -463,7 +463,6 @@ func (n *NetworkHarness) EnsureConnected(ctx context.Context, a, b *HarnessNode) err := n.connect(ctx, req, a) switch { - // Request was successful, wait for both to display the // connection. case err == nil: diff --git a/lntest/itest/lnd_channel_backup_test.go b/lntest/itest/lnd_channel_backup_test.go index 4a394c2d..5a8bf87d 100644 --- a/lntest/itest/lnd_channel_backup_test.go +++ b/lntest/itest/lnd_channel_backup_test.go @@ -364,6 +364,7 @@ func testChannelBackupRestore(net *lntest.NetworkHarness, t *harnessTest) { // ann is updated? for _, testCase := range testCases { + testCase := testCase success := t.t.Run(testCase.name, func(t *testing.T) { h := newHarnessTest(t, net) @@ -543,7 +544,7 @@ func testChannelBackupUpdates(net *lntest.NetworkHarness, t *harnessTest) { chanPoint := chanPoints[i] - ctxt, _ = context.WithTimeout(ctxb, channelCloseTimeout) + ctxt, _ := context.WithTimeout(ctxb, channelCloseTimeout) closeChannelAndAssert( ctxt, t, net, net.Alice, chanPoint, forceClose, ) diff --git a/lntest/itest/lnd_forward_interceptor_test.go b/lntest/itest/lnd_forward_interceptor_test.go index ed36f879..6c54756f 100644 --- a/lntest/itest/lnd_forward_interceptor_test.go +++ b/lntest/itest/lnd_forward_interceptor_test.go @@ -120,7 +120,7 @@ func testForwardInterceptor(net *lntest.NetworkHarness, t *harnessTest) { t.t.Errorf("expected payment to fail, instead got %v", attempt.Status) } - // For settle and resume we make sure the payment is successfull. + // For settle and resume we make sure the payment is successful. case routerrpc.ResolveHoldForwardAction_SETTLE: fallthrough @@ -164,7 +164,7 @@ func testForwardInterceptor(net *lntest.NetworkHarness, t *harnessTest) { } // For all other packets we resolve according to the test case. - interceptor.Send(&routerrpc.ForwardHtlcInterceptResponse{ + _ = interceptor.Send(&routerrpc.ForwardHtlcInterceptResponse{ IncomingCircuitKey: request.IncomingCircuitKey, Action: testCase.interceptorAction, Preimage: testCase.invoice.RPreimage, diff --git a/lntest/itest/lnd_macaroons_test.go b/lntest/itest/lnd_macaroons_test.go index 8dee9572..93ef10b0 100644 --- a/lntest/itest/lnd_macaroons_test.go +++ b/lntest/itest/lnd_macaroons_test.go @@ -108,14 +108,14 @@ func testMacaroonAuthentication(net *lntest.NetworkHarness, t *harnessTest) { testNode.ReadMacPath(), defaultTimeout, ) require.NoError(t, err) - invalidIpAddrMac, err := macaroons.AddConstraints( + invalidIPAddrMac, err := macaroons.AddConstraints( readonlyMac, macaroons.IPLockConstraint( "1.1.1.1", ), ) require.NoError(t, err) cleanup, client := macaroonClient( - t, testNode, invalidIpAddrMac, + t, testNode, invalidIPAddrMac, ) defer cleanup() _, err = client.GetInfo(ctxt, infoReq) diff --git a/lntest/itest/lnd_mpp_test.go b/lntest/itest/lnd_mpp_test.go index c240a2b0..e213d981 100644 --- a/lntest/itest/lnd_mpp_test.go +++ b/lntest/itest/lnd_mpp_test.go @@ -23,7 +23,7 @@ func testSendToRouteMultiPath(net *lntest.NetworkHarness, t *harnessTest) { ctx := newMppTestContext(t, net) defer ctx.shutdownNodes() - // To ensure the payment goes through seperate paths, we'll set a + // To ensure the payment goes through separate paths, we'll set a // channel size that can only carry one shard at a time. We'll divide // the payment into 3 shards. const ( diff --git a/lntest/itest/lnd_multi-hop-payments_test.go b/lntest/itest/lnd_multi-hop-payments_test.go index 5c546a7c..ed8f25d8 100644 --- a/lntest/itest/lnd_multi-hop-payments_test.go +++ b/lntest/itest/lnd_multi-hop-payments_test.go @@ -168,7 +168,7 @@ func testMultiHopPayments(net *lntest.NetworkHarness, t *harnessTest) { // Set the fee policies of the Alice -> Bob and the Dave -> Alice // channel edges to relatively large non default values. This makes it // possible to pick up more subtle fee calculation errors. - maxHtlc := uint64(calculateMaxHtlc(chanAmt)) + maxHtlc := calculateMaxHtlc(chanAmt) const aliceBaseFeeSat = 1 const aliceFeeRatePPM = 100000 updateChannelPolicy( diff --git a/lntest/itest/lnd_multi-hop_htlc_local_timeout_test.go b/lntest/itest/lnd_multi-hop_htlc_local_timeout_test.go index 6e61b458..8e55a744 100644 --- a/lntest/itest/lnd_multi-hop_htlc_local_timeout_test.go +++ b/lntest/itest/lnd_multi-hop_htlc_local_timeout_test.go @@ -93,11 +93,7 @@ func testMultiHopHtlcLocalTimeout(net *lntest.NetworkHarness, t *harnessTest, nodes := []*lntest.HarnessNode{alice, bob, carol} err = wait.Predicate(func() bool { predErr = assertActiveHtlcs(nodes, dustPayHash, payHash) - if predErr != nil { - return false - } - - return true + return predErr == nil }, time.Second*15) if err != nil { t.Fatalf("htlc mismatch: %v", predErr) @@ -152,11 +148,7 @@ func testMultiHopHtlcLocalTimeout(net *lntest.NetworkHarness, t *harnessTest, nodes = []*lntest.HarnessNode{alice} err = wait.Predicate(func() bool { predErr = assertActiveHtlcs(nodes, payHash) - if predErr != nil { - return false - } - - return true + return predErr == nil }, time.Second*15) if err != nil { t.Fatalf("htlc mismatch: %v", predErr) @@ -240,10 +232,7 @@ func testMultiHopHtlcLocalTimeout(net *lntest.NetworkHarness, t *harnessTest, nodes = []*lntest.HarnessNode{alice} err = wait.Predicate(func() bool { predErr = assertNumActiveHtlcs(nodes, 0) - if predErr != nil { - return false - } - return true + return predErr == nil }, time.Second*15) if err != nil { t.Fatalf("alice's channel still has active htlc's: %v", predErr) diff --git a/lntest/itest/lnd_multi-hop_htlc_receiver_chain_claim_test.go b/lntest/itest/lnd_multi-hop_htlc_receiver_chain_claim_test.go index fb118940..1e022844 100644 --- a/lntest/itest/lnd_multi-hop_htlc_receiver_chain_claim_test.go +++ b/lntest/itest/lnd_multi-hop_htlc_receiver_chain_claim_test.go @@ -136,7 +136,7 @@ func testMultiHopReceiverChainClaim(net *lntest.NetworkHarness, t *harnessTest, if c == commitTypeAnchors { expectedTxes = 2 } - txes, err := getNTxsFromMempool( + _, err = getNTxsFromMempool( net.Miner.Node, expectedTxes, minerMempoolTimeout, ) if err != nil { @@ -178,7 +178,7 @@ func testMultiHopReceiverChainClaim(net *lntest.NetworkHarness, t *harnessTest, if c == commitTypeAnchors { expectedTxes = 3 } - txes, err = getNTxsFromMempool(net.Miner.Node, + txes, err := getNTxsFromMempool(net.Miner.Node, expectedTxes, minerMempoolTimeout) if err != nil { t.Fatalf("transactions not found in mempool: %v", err) diff --git a/lntest/itest/lnd_multi-hop_local_force_close_on_chain_htlc_timeout_test.go b/lntest/itest/lnd_multi-hop_local_force_close_on_chain_htlc_timeout_test.go index 449c4573..8eb5b97f 100644 --- a/lntest/itest/lnd_multi-hop_local_force_close_on_chain_htlc_timeout_test.go +++ b/lntest/itest/lnd_multi-hop_local_force_close_on_chain_htlc_timeout_test.go @@ -67,11 +67,7 @@ func testMultiHopLocalForceCloseOnChainHtlcTimeout(net *lntest.NetworkHarness, nodes := []*lntest.HarnessNode{alice, bob, carol} err = wait.Predicate(func() bool { predErr = assertActiveHtlcs(nodes, payHash) - if predErr != nil { - return false - } - - return true + return predErr == nil }, time.Second*15) if err != nil { t.Fatalf("htlc mismatch: %v", err) @@ -201,10 +197,7 @@ func testMultiHopLocalForceCloseOnChainHtlcTimeout(net *lntest.NetworkHarness, nodes = []*lntest.HarnessNode{alice} err = wait.Predicate(func() bool { predErr = assertNumActiveHtlcs(nodes, 0) - if predErr != nil { - return false - } - return true + return predErr == nil }, time.Second*15) if err != nil { t.Fatalf("alice's channel still has active htlc's: %v", predErr) diff --git a/lntest/itest/lnd_multi-hop_remote_force_close_on_chain_htlc_timeout_test.go b/lntest/itest/lnd_multi-hop_remote_force_close_on_chain_htlc_timeout_test.go index 53e56227..19d92165 100644 --- a/lntest/itest/lnd_multi-hop_remote_force_close_on_chain_htlc_timeout_test.go +++ b/lntest/itest/lnd_multi-hop_remote_force_close_on_chain_htlc_timeout_test.go @@ -68,11 +68,7 @@ func testMultiHopRemoteForceCloseOnChainHtlcTimeout(net *lntest.NetworkHarness, nodes := []*lntest.HarnessNode{alice, bob, carol} err = wait.Predicate(func() bool { predErr = assertActiveHtlcs(nodes, payHash) - if predErr != nil { - return false - } - - return true + return predErr == nil }, time.Second*15) if err != nil { t.Fatalf("htlc mismatch: %v", predErr) @@ -210,10 +206,7 @@ func testMultiHopRemoteForceCloseOnChainHtlcTimeout(net *lntest.NetworkHarness, nodes = []*lntest.HarnessNode{alice} err = wait.Predicate(func() bool { predErr = assertNumActiveHtlcs(nodes, 0) - if predErr != nil { - return false - } - return true + return predErr == nil }, time.Second*15) if err != nil { t.Fatalf("alice's channel still has active htlc's: %v", predErr) diff --git a/lntest/itest/lnd_multi-hop_test.go b/lntest/itest/lnd_multi-hop_test.go index 29817461..3707ae11 100644 --- a/lntest/itest/lnd_multi-hop_test.go +++ b/lntest/itest/lnd_multi-hop_test.go @@ -71,6 +71,7 @@ func testMultiHopHtlcClaims(net *lntest.NetworkHarness, t *harnessTest) { for _, commitType := range commitTypes { testName := fmt.Sprintf("committype=%v", commitType.String()) + commitType := commitType success := t.t.Run(testName, func(t *testing.T) { ht := newHarnessTest(t, net) diff --git a/lntest/itest/lnd_network_test.go b/lntest/itest/lnd_network_test.go index 8ce168b0..a1d69f53 100644 --- a/lntest/itest/lnd_network_test.go +++ b/lntest/itest/lnd_network_test.go @@ -79,7 +79,7 @@ func assertTimeoutError(ctxt context.Context, t *harnessTest, ctxt, cancel := context.WithTimeout(ctxt, defaultTimeout) defer cancel() - err := connect(node, ctxt, req) + err := connect(ctxt, node, req) // a DeadlineExceeded error will appear in the context if the above // ctxtTimeout value is reached. @@ -92,7 +92,7 @@ func assertTimeoutError(ctxt context.Context, t *harnessTest, ) } -func connect(node *lntest.HarnessNode, ctxt context.Context, +func connect(ctxt context.Context, node *lntest.HarnessNode, req *lnrpc.ConnectPeerRequest) error { syncTimeout := time.After(15 * time.Second) diff --git a/lntest/itest/lnd_psbt_test.go b/lntest/itest/lnd_psbt_test.go index b2affc03..69af9589 100644 --- a/lntest/itest/lnd_psbt_test.go +++ b/lntest/itest/lnd_psbt_test.go @@ -400,7 +400,7 @@ func receiveChanUpdate(ctx context.Context, errChan := make(chan error) go func() { // Consume one message. This will block until the message is - // recieved. + // received. resp, err := stream.Recv() if err != nil { errChan <- err diff --git a/lntest/itest/lnd_rest_api_test.go b/lntest/itest/lnd_rest_api_test.go index f0b402d8..573d0b43 100644 --- a/lntest/itest/lnd_rest_api_test.go +++ b/lntest/itest/lnd_rest_api_test.go @@ -49,9 +49,9 @@ var ( resultPattern = regexp.MustCompile("{\"result\":(.*)}") ) -// testRestApi tests that the most important features of the REST API work +// testRestAPI tests that the most important features of the REST API work // correctly. -func testRestApi(net *lntest.NetworkHarness, ht *harnessTest) { +func testRestAPI(net *lntest.NetworkHarness, ht *harnessTest) { testCases := []struct { name string run func(*testing.T, *lntest.HarnessNode, *lntest.HarnessNode) @@ -396,6 +396,7 @@ func testRestApi(net *lntest.NetworkHarness, ht *harnessTest) { } for _, tc := range testCases { + tc := tc ht.t.Run(tc.name, func(t *testing.T) { tc.run(t, net.Alice, net.Bob) }) @@ -486,10 +487,11 @@ func openWebSocket(node *lntest.HarnessNode, url, method string, fullURL := fmt.Sprintf( "wss://%s%s?method=%s", node.Cfg.RESTAddr(), url, method, ) - conn, _, err := webSocketDialer.Dial(fullURL, header) + conn, resp, err := webSocketDialer.Dial(fullURL, header) if err != nil { return nil, err } + defer func() { _ = resp.Body.Close() }() // Send the given request message as the first message on the socket. reqMsg, err := jsonMarshaler.MarshalToString(req) diff --git a/lntest/itest/lnd_single_hop_invoice_test.go b/lntest/itest/lnd_single_hop_invoice_test.go index 58c26d0b..83d914a1 100644 --- a/lntest/itest/lnd_single_hop_invoice_test.go +++ b/lntest/itest/lnd_single_hop_invoice_test.go @@ -40,7 +40,6 @@ func testSingleHopInvoice(net *lntest.NetworkHarness, t *harnessTest) { RPreimage: preimage, Value: paymentAmt, } - ctxt, _ = context.WithTimeout(ctxt, defaultTimeout) invoiceResp, err := net.Bob.AddInvoice(ctxb, invoice) if err != nil { t.Fatalf("unable to add invoice: %v", err) diff --git a/lntest/itest/lnd_test.go b/lntest/itest/lnd_test.go index 49254d30..6b3ba1f5 100644 --- a/lntest/itest/lnd_test.go +++ b/lntest/itest/lnd_test.go @@ -1187,7 +1187,7 @@ func basicChannelFundingTest(t *harnessTest, net *lntest.NetworkHarness, // Finally, immediately close the channel. This function will // also block until the channel is closed and will additionally // assert the relevant channel closing post conditions. - ctxt, _ = context.WithTimeout(ctxb, channelCloseTimeout) + ctxt, _ := context.WithTimeout(ctxb, channelCloseTimeout) closeChannelAndAssert(ctxt, t, net, alice, chanPoint, false) } @@ -1253,6 +1253,8 @@ test: carolCommitType, daveCommitType) ht := t + carolCommitType := carolCommitType + daveCommitType := daveCommitType success := t.t.Run(testName, func(t *testing.T) { carolChannel, daveChannel, closeChan, err := basicChannelFundingTest( ht, net, carol, dave, nil, @@ -1454,7 +1456,7 @@ func testPaymentFollowingChannelOpen(net *lntest.NetworkHarness, t *harnessTest) ctxb := context.Background() const paymentAmt = btcutil.Amount(100) - channelCapacity := btcutil.Amount(paymentAmt * 1000) + channelCapacity := paymentAmt * 1000 // We first establish a channel between Alice and Bob. ctxt, cancel := context.WithTimeout(ctxb, channelOpenTimeout) @@ -2148,7 +2150,7 @@ func testUpdateChannelPolicy(net *lntest.NetworkHarness, t *harnessTest) { baseFee = int64(800) feeRate = int64(123) timeLockDelta = uint32(22) - maxHtlc = maxHtlc * 2 + maxHtlc *= 2 expectedPolicy.FeeBaseMsat = baseFee expectedPolicy.FeeRateMilliMsat = testFeeBase * feeRate @@ -2762,7 +2764,7 @@ func testChannelFundingPersistence(net *lntest.NetworkHarness, t *harnessTest) { // Assert that our wallet has our opening transaction with a label // that does not have a channel ID set yet, because we have not // reached our required confirmations. - tx := findTxAtHeight(ctxt, t, height, fundingTxStr, net, net.Alice) + tx := findTxAtHeight(ctxt, t, height, fundingTxStr, net.Alice) // At this stage, we expect the transaction to be labelled, but not with // our channel ID because our transaction has not yet confirmed. @@ -2793,7 +2795,7 @@ func testChannelFundingPersistence(net *lntest.NetworkHarness, t *harnessTest) { } // Re-lookup our transaction in the block that it confirmed in. - tx = findTxAtHeight(ctxt, t, height, fundingTxStr, net, net.Alice) + tx = findTxAtHeight(ctxt, t, height, fundingTxStr, net.Alice) // Create an additional check for our channel assertion that will // check that our label is as expected. @@ -2837,8 +2839,7 @@ func testChannelFundingPersistence(net *lntest.NetworkHarness, t *harnessTest) { // of at the target height, and finds and returns the tx with the target txid, // failing if it is not found. func findTxAtHeight(ctx context.Context, t *harnessTest, height int32, - target string, net *lntest.NetworkHarness, - node *lntest.HarnessNode) *lnrpc.Transaction { + target string, node *lntest.HarnessNode) *lnrpc.Transaction { txns, err := node.LightningClient.GetTransactions( ctx, &lnrpc.GetTransactionsRequest{ @@ -3208,6 +3209,7 @@ func testChannelForceClosure(net *lntest.NetworkHarness, t *harnessTest) { for _, channelType := range commitTypes { testName := fmt.Sprintf("committype=%v", channelType) + channelType := channelType success := t.t.Run(testName, func(t *testing.T) { ht := newHarnessTest(t, net) @@ -4123,7 +4125,7 @@ func channelForceClosureTest(net *lntest.NetworkHarness, t *harnessTest, TxidStr: output.Hash.String(), OutputIndex: output.Index, }, - AmountSat: uint64(htlcLessFees), + AmountSat: htlcLessFees, } } @@ -4702,13 +4704,12 @@ func testListChannels(net *lntest.NetworkHarness, t *harnessTest) { defer shutdownAndAssert(net, t, bob) // Connect Alice to Bob. - ctxt, _ := context.WithTimeout(ctxb, defaultTimeout) if err := net.ConnectNodes(ctxb, alice, bob); err != nil { t.Fatalf("unable to connect alice to bob: %v", err) } // Give Alice some coins so she can fund a channel. - ctxt, _ = context.WithTimeout(ctxb, defaultTimeout) + ctxt, _ := context.WithTimeout(ctxb, defaultTimeout) err = net.SendCoins(ctxt, btcutil.SatoshiPerBitcoin, alice) if err != nil { t.Fatalf("unable to send coins to alice: %v", err) @@ -5417,7 +5418,7 @@ func testSingleHopSendToRouteCase(net *lntest.NetworkHarness, t *harnessTest, i, p.PaymentRequest) } - // Assert the payment ammount is correct. + // Assert the payment amount is correct. if p.ValueSat != paymentAmtSat { t.Fatalf("incorrect payment amt for payment %d, "+ "want: %d, got: %d", @@ -9284,7 +9285,7 @@ func assertDLPExecuted(net *lntest.NetworkHarness, t *harnessTest, } // Generate a single block, which should confirm the closing tx. - block := mineBlocks(t, net, 1, expectedTxes)[0] + _ = mineBlocks(t, net, 1, expectedTxes)[0] // Dave should sweep his funds immediately, as they are not timelocked. // We also expect Dave to sweep his anchor, if present. @@ -9305,7 +9306,7 @@ func assertDLPExecuted(net *lntest.NetworkHarness, t *harnessTest, assertNumPendingChannels(t, carol, 0, 1) // Mine the sweep tx. - block = mineBlocks(t, net, 1, expectedTxes)[0] + _ = mineBlocks(t, net, 1, expectedTxes)[0] // Now Dave should consider the channel fully closed. assertNumPendingChannels(t, dave, 0, 0) @@ -9334,7 +9335,7 @@ func assertDLPExecuted(net *lntest.NetworkHarness, t *harnessTest, if err != nil { t.Fatalf("unable to find Carol's sweep tx in mempool: %v", err) } - block = mineBlocks(t, net, 1, 1)[0] + block := mineBlocks(t, net, 1, 1)[0] assertTxInBlock(t, block, carolSweep) // Now the channel should be fully closed also from Carol's POV. @@ -11050,11 +11051,7 @@ func testSwitchCircuitPersistence(net *lntest.NetworkHarness, t *harnessTest) { // Ensure all nodes in the network still have 5 outstanding htlcs. err = wait.Predicate(func() bool { predErr = assertNumActiveHtlcs(nodes, numPayments) - if predErr != nil { - return false - } - return true - + return predErr == nil }, time.Second*15) if err != nil { t.Fatalf("htlc mismatch: %v", predErr) @@ -11365,10 +11362,7 @@ func testSwitchOfflineDelivery(net *lntest.NetworkHarness, t *harnessTest) { // for the duration of the interval. err = wait.Invariant(func() bool { predErr = assertNumActiveHtlcs(nodes, numPayments) - if predErr != nil { - return false - } - return true + return predErr == nil }, time.Second*2) if err != nil { t.Fatalf("htlc change: %v", predErr) @@ -11392,10 +11386,7 @@ func testSwitchOfflineDelivery(net *lntest.NetworkHarness, t *harnessTest) { carolNode := []*lntest.HarnessNode{carol} err = wait.Predicate(func() bool { predErr = assertNumActiveHtlcs(carolNode, 0) - if predErr != nil { - return false - } - return true + return predErr == nil }, time.Second*15) if err != nil { t.Fatalf("htlc mismatch: %v", predErr) @@ -11683,7 +11674,6 @@ func testSwitchOfflineDeliveryPersistence(net *lntest.NetworkHarness, t *harness // Disconnect the two intermediaries, Alice and Dave, by shutting down // Alice. - ctxt, _ = context.WithTimeout(ctxb, defaultTimeout) if err := net.StopNode(net.Alice); err != nil { t.Fatalf("unable to shutdown alice: %v", err) } @@ -11713,10 +11703,7 @@ func testSwitchOfflineDeliveryPersistence(net *lntest.NetworkHarness, t *harness } predErr = assertNumActiveHtlcsChanPoint(dave, carolFundPoint, 0) - if predErr != nil { - return false - } - return true + return predErr == nil }, time.Second*15) if err != nil { t.Fatalf("htlc mismatch: %v", predErr) @@ -12021,7 +12008,6 @@ func testSwitchOfflineDeliveryOutgoingOffline( // Disconnect the two intermediaries, Alice and Dave, so that when carol // restarts, the response will be held by Dave. - ctxt, _ = context.WithTimeout(ctxb, defaultTimeout) if err := net.StopNode(net.Alice); err != nil { t.Fatalf("unable to shutdown alice: %v", err) } @@ -12042,11 +12028,7 @@ func testSwitchOfflineDeliveryOutgoingOffline( } predErr = assertNumActiveHtlcsChanPoint(dave, carolFundPoint, 0) - if predErr != nil { - return false - } - - return true + return predErr == nil }, time.Second*15) if err != nil { t.Fatalf("htlc mismatch: %v", predErr) @@ -12930,10 +12912,10 @@ func testAbandonChannel(net *lntest.NetworkHarness, t *harnessTest) { if err != nil { t.Fatalf("unable to list pending channels: %v", err) } - if len(alicePendingList.PendingClosingChannels) != 0 { + if len(alicePendingList.PendingClosingChannels) != 0 { //nolint:staticcheck t.Fatalf("alice should only have no pending closing channels, "+ "instead she has %v", - len(alicePendingList.PendingClosingChannels)) + len(alicePendingList.PendingClosingChannels)) //nolint:staticcheck } if len(alicePendingList.PendingForceClosingChannels) != 0 { t.Fatalf("alice should only have no pending force closing "+ @@ -13558,7 +13540,7 @@ func testHoldInvoicePersistence(net *lntest.NetworkHarness, t *harnessTest) { // Assert terminal payment state. if i%2 == 0 { if payment.Status != lnrpc.Payment_SUCCEEDED { - t.Fatalf("state not suceeded : %v", + t.Fatalf("state not succeeded : %v", payment.Status) } } else { diff --git a/lntest/itest/lnd_test_list_on_test.go b/lntest/itest/lnd_test_list_on_test.go index e4cb916e..98910d22 100644 --- a/lntest/itest/lnd_test_list_on_test.go +++ b/lntest/itest/lnd_test_list_on_test.go @@ -264,7 +264,7 @@ var testsCases = []*testCase{ }, { name: "REST API", - test: testRestApi, + test: testRestAPI, }, { name: "intercept forwarded htlc packets", diff --git a/lntest/itest/test_harness.go b/lntest/itest/test_harness.go index daf089c0..a3c47528 100644 --- a/lntest/itest/test_harness.go +++ b/lntest/itest/test_harness.go @@ -101,8 +101,6 @@ func (h *harnessTest) RunTestCase(testCase *testCase) { }() testCase.test(h.lndHarness, h) - - return } func (h *harnessTest) Logf(format string, args ...interface{}) { From 9f7d8dd92e9c1f9e7cf482e4f9bdbf9149d77684 Mon Sep 17 00:00:00 2001 From: Oliver Gugger Date: Fri, 4 Sep 2020 13:47:56 +0200 Subject: [PATCH 5/6] lntest: make compilable without subserver build tags To make it possible to compile the itests together with the other tests, we don't want to use anything from the optional subservers. --- lntest/itest/lnd_test.go | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/lntest/itest/lnd_test.go b/lntest/itest/lnd_test.go index 6b3ba1f5..4149802a 100644 --- a/lntest/itest/lnd_test.go +++ b/lntest/itest/lnd_test.go @@ -13124,7 +13124,8 @@ func testSweepAllCoins(net *lntest.NetworkHarness, t *harnessTest) { // Our error will be wrapped in a rpc error, so we check that it // contains the error we expect. - if !strings.Contains(err.Error(), walletrpc.ErrZeroLabel.Error()) { + errZeroLabel := "cannot label transaction with empty label" + if !strings.Contains(err.Error(), errZeroLabel) { t.Fatalf("expected: zero label error, got: %v", err) } From 1558edbc3cee850a6493b8764faa153d7fb09beb Mon Sep 17 00:00:00 2001 From: Oliver Gugger Date: Fri, 4 Sep 2020 14:10:03 +0200 Subject: [PATCH 6/6] travis+lint: fix memory usage There is a setting to control how often the garbage collector is run. Apparently this is a tradeoff between CPU and memory usage. If we can limit the memory being used in that way, this allows us to use multiple worker again, so overall this shouldn't be much slower than before. --- .golangci.yml | 2 +- .travis.yml | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/.golangci.yml b/.golangci.yml index 1ffef44b..ee9246b2 100644 --- a/.golangci.yml +++ b/.golangci.yml @@ -1,6 +1,6 @@ run: # timeout for analysis - deadline: 4m + deadline: 10m # Skip autogenerated files for mobile. skip-files: diff --git a/.travis.yml b/.travis.yml index fd3f5446..ad87c799 100644 --- a/.travis.yml +++ b/.travis.yml @@ -44,8 +44,8 @@ jobs: # in a GitHub Workflow. - make unit pkg=... case=_NONE_ - # Step 3: Lint go code. Limit to 1 worker to reduce memory usage. - - make lint workers=1 + # Step 3: Lint go code. Invoke GC more often to reduce memory usage. + - GOGC=30 make lint - stage: Integration Test name: Btcd Integration