From 933d84273a2802da169b69c100930ee563833dee Mon Sep 17 00:00:00 2001 From: yyforyongyu Date: Thu, 3 Sep 2020 02:38:54 +0800 Subject: [PATCH 1/4] itest: require no error when tearing down miners --- lntest/itest/lnd_test.go | 11 +++++++++-- 1 file changed, 9 insertions(+), 2 deletions(-) diff --git a/lntest/itest/lnd_test.go b/lntest/itest/lnd_test.go index 94dbf293..8a094b3b 100644 --- a/lntest/itest/lnd_test.go +++ b/lntest/itest/lnd_test.go @@ -2477,7 +2477,12 @@ func testOpenChannelAfterReorg(net *lntest.NetworkHarness, t *harnessTest) { if err := tempMiner.SetUp(false, 0); err != nil { t.Fatalf("unable to set up mining node: %v", err) } - defer tempMiner.TearDown() + defer func() { + require.NoError( + t.t, tempMiner.TearDown(), + "failed to tear down temp miner", + ) + }() // We start by connecting the new miner to our original miner, // such that it will sync to our original chain. @@ -14414,7 +14419,9 @@ func TestLightningNetworkDaemon(t *testing.T) { ht.Fatalf("unable to create mining node: %v", err) } defer func() { - miner.TearDown() + require.NoError( + t, miner.TearDown(), "failed to tear down miner", + ) // After shutting down the miner, we'll make a copy of the log // file before deleting the temporary log dir. From 724f6e0969330677ebf5e0fb78ba36086548af3f Mon Sep 17 00:00:00 2001 From: yyforyongyu Date: Thu, 3 Sep 2020 03:15:04 +0800 Subject: [PATCH 2/4] itest: require no error when cleaning up chainbackends --- lntest/bitcoind.go | 24 ++++++++++++++++++------ lntest/btcd.go | 20 +++++++++++++++----- lntest/itest/lnd_test.go | 6 +++++- lntest/neutrino.go | 4 ++-- 4 files changed, 40 insertions(+), 14 deletions(-) diff --git a/lntest/bitcoind.go b/lntest/bitcoind.go index 0a73c86e..86d59ef4 100644 --- a/lntest/bitcoind.go +++ b/lntest/bitcoind.go @@ -3,6 +3,7 @@ package lntest import ( + "errors" "fmt" "io/ioutil" "math/rand" @@ -71,7 +72,7 @@ func (b BitcoindBackendConfig) Name() string { // NewBackend starts a bitcoind node and returns a BitoindBackendConfig for // that node. func NewBackend(miner string, netParams *chaincfg.Params) ( - *BitcoindBackendConfig, func(), error) { + *BitcoindBackendConfig, func() error, error) { if netParams != &chaincfg.RegressionNetParams { return nil, nil, fmt.Errorf("only regtest supported") @@ -121,21 +122,32 @@ func NewBackend(miner string, netParams *chaincfg.Params) ( return nil, nil, fmt.Errorf("couldn't start bitcoind: %v", err) } - cleanUp := func() { + cleanUp := func() error { bitcoind.Process.Kill() bitcoind.Wait() + var errStr string // After shutting down the chain backend, we'll make a copy of // the log file before deleting the temporary log dir. err := CopyFile("./output_bitcoind_chainbackend.log", logFile) if err != nil { - fmt.Printf("unable to copy file: %v\n", err) + errStr += fmt.Sprintf("unable to copy file: %v\n", err) } if err = os.RemoveAll(logDir); err != nil { - fmt.Printf("Cannot remove dir %s: %v\n", logDir, err) + errStr += fmt.Sprintf( + "cannot remove dir %s: %v\n", logDir, err, + ) } - - os.RemoveAll(tempBitcoindDir) + if err := os.RemoveAll(tempBitcoindDir); err != nil { + errStr += fmt.Sprintf( + "cannot remove dir %s: %v\n", + tempBitcoindDir, err, + ) + } + if errStr != "" { + return errors.New(errStr) + } + return nil } // Allow process to start. diff --git a/lntest/btcd.go b/lntest/btcd.go index 2322d8ee..3b75b7bc 100644 --- a/lntest/btcd.go +++ b/lntest/btcd.go @@ -4,6 +4,7 @@ package lntest import ( "encoding/hex" + "errors" "fmt" "os" @@ -72,7 +73,7 @@ func (b BtcdBackendConfig) Name() string { // that node. miner should be set to the P2P address of the miner to connect // to. func NewBackend(miner string, netParams *chaincfg.Params) ( - *BtcdBackendConfig, func(), error) { + *BtcdBackendConfig, func() error, error) { args := []string{ "--rejectnonstd", @@ -98,19 +99,28 @@ func NewBackend(miner string, netParams *chaincfg.Params) ( minerAddr: miner, } - cleanUp := func() { - chainBackend.TearDown() + cleanUp := func() error { + var errStr string + if err := chainBackend.TearDown(); err != nil { + errStr += err.Error() + "\n" + } // After shutting down the chain backend, we'll make a copy of // the log file before deleting the temporary log dir. logFile := logDir + "/" + netParams.Name + "/btcd.log" err := CopyFile("./output_btcd_chainbackend.log", logFile) if err != nil { - fmt.Printf("unable to copy file: %v\n", err) + errStr += fmt.Sprintf("unable to copy file: %v\n", err) } if err = os.RemoveAll(logDir); err != nil { - fmt.Printf("Cannot remove dir %s: %v\n", logDir, err) + errStr += fmt.Sprintf( + "cannot remove dir %s: %v\n", logDir, err, + ) } + if errStr != "" { + return errors.New(errStr) + } + return nil } return bd, cleanUp, nil diff --git a/lntest/itest/lnd_test.go b/lntest/itest/lnd_test.go index 8a094b3b..a8589450 100644 --- a/lntest/itest/lnd_test.go +++ b/lntest/itest/lnd_test.go @@ -14445,7 +14445,11 @@ func TestLightningNetworkDaemon(t *testing.T) { if err != nil { ht.Fatalf("unable to start backend: %v", err) } - defer cleanUp() + defer func() { + require.NoError( + t, cleanUp(), "failed to clean up chain backend", + ) + }() if err := miner.SetUp(true, 50); err != nil { ht.Fatalf("unable to set up mining node: %v", err) diff --git a/lntest/neutrino.go b/lntest/neutrino.go index 1a5497b9..25f2af24 100644 --- a/lntest/neutrino.go +++ b/lntest/neutrino.go @@ -44,12 +44,12 @@ func (b NeutrinoBackendConfig) Name() string { // NewBackend starts and returns a NeutrinoBackendConfig for the node. func NewBackend(miner string, _ *chaincfg.Params) ( - *NeutrinoBackendConfig, func(), error) { + *NeutrinoBackendConfig, func() error, error) { bd := &NeutrinoBackendConfig{ minerAddr: miner, } - cleanUp := func() {} + cleanUp := func() error { return nil } return bd, cleanUp, nil } From 567fa9ed107939e89220d9a7f50436b78aeee045 Mon Sep 17 00:00:00 2001 From: yyforyongyu Date: Thu, 3 Sep 2020 18:59:08 +0800 Subject: [PATCH 3/4] itest: disable node retrying to connect to miner MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Previously the chainbackend connected to the miner using a permanent connection, which would retry the connection when it’s disconnected. It would leave multiple connections between the chainbackend and the miner, causing difficulties in debugging. Currently, there are two occasions when disconnection happens. One happens when running the open channel reorg test, the miner is connected/disconnected multiple times on purpose. The other happens when the chainbackend receives a MSG_WITNESS_TX type from the miner, which would be considered as an invalid type and disconnects the miner. With the latter one being fixed in btcd, the chainbackend will still be disconnected from the miner if it reaches the ban score by requesting too many notfound messages in a short time which is why the `--nobanning` flag is added. --- lntest/bitcoind.go | 8 -------- lntest/btcd.go | 13 ++++++++----- lntest/itest/lnd_test.go | 8 ++++++++ lntest/neutrino.go | 2 +- 4 files changed, 17 insertions(+), 14 deletions(-) diff --git a/lntest/bitcoind.go b/lntest/bitcoind.go index 86d59ef4..56670892 100644 --- a/lntest/bitcoind.go +++ b/lntest/bitcoind.go @@ -174,14 +174,6 @@ func NewBackend(miner string, netParams *chaincfg.Params) ( err) } - // We start by adding the miner to the bitcoind addnode list. We do - // this instead of connecting using command line flags, because it will - // allow us to disconnect the miner using the AddNode command later. - if err := client.AddNode(miner, rpcclient.ANAdd); err != nil { - cleanUp() - return nil, nil, fmt.Errorf("unable to add node: %v", err) - } - bd := BitcoindBackendConfig{ rpcHost: rpcHost, rpcUser: rpcUser, diff --git a/lntest/btcd.go b/lntest/btcd.go index 3b75b7bc..b3ec7b45 100644 --- a/lntest/btcd.go +++ b/lntest/btcd.go @@ -17,11 +17,11 @@ import ( // logDir is the name of the temporary log directory. const logDir = "./.backendlogs" -// perm is used to signal we want to establish a permanent connection using the +// temp is used to signal we want to establish a temporary connection using the // btcd Node API. // // NOTE: Cannot be const, since the node API expects a reference. -var perm = "perm" +var temp = "temp" // BtcdBackendConfig is an implementation of the BackendConfig interface // backed by a btcd node. @@ -56,12 +56,12 @@ func (b BtcdBackendConfig) GenArgs() []string { // ConnectMiner is called to establish a connection to the test miner. func (b BtcdBackendConfig) ConnectMiner() error { - return b.harness.Node.Node(btcjson.NConnect, b.minerAddr, &perm) + return b.harness.Node.Node(btcjson.NConnect, b.minerAddr, &temp) } // DisconnectMiner is called to disconnect the miner. func (b BtcdBackendConfig) DisconnectMiner() error { - return b.harness.Node.Node(btcjson.NRemove, b.minerAddr, &perm) + return b.harness.Node.Node(btcjson.NDisconnect, b.minerAddr, &temp) } // Name returns the name of the backend type. @@ -81,8 +81,11 @@ func NewBackend(miner string, netParams *chaincfg.Params) ( "--trickleinterval=100ms", "--debuglevel=debug", "--logdir=" + logDir, - "--connect=" + miner, "--nowinservice", + // The miner will get banned and disconnected from the node if + // its requested data are not found. We add a nobanning flag to + // make sure they stay connected if it happens. + "--nobanning", } chainBackend, err := rpctest.New(netParams, nil, args) if err != nil { diff --git a/lntest/itest/lnd_test.go b/lntest/itest/lnd_test.go index a8589450..0d151b71 100644 --- a/lntest/itest/lnd_test.go +++ b/lntest/itest/lnd_test.go @@ -2467,6 +2467,7 @@ func testOpenChannelAfterReorg(net *lntest.NetworkHarness, t *harnessTest) { "--rejectnonstd", "--txindex", "--nowinservice", + "--nobanning", } tempMiner, err := rpctest.New( harnessNetParams, &rpcclient.NotificationHandlers{}, args, @@ -14407,6 +14408,7 @@ func TestLightningNetworkDaemon(t *testing.T) { "--logdir=" + minerLogDir, "--trickleinterval=100ms", "--nowinservice", + "--nobanning", } handlers := &rpcclient.NotificationHandlers{ OnTxAccepted: func(hash *chainhash.Hash, amt btcutil.Amount) { @@ -14458,6 +14460,12 @@ func TestLightningNetworkDaemon(t *testing.T) { ht.Fatalf("unable to request transaction notifications: %v", err) } + // Connect chainbackend to miner. + require.NoError( + t, chainBackend.ConnectMiner(), + "failed to connect to miner", + ) + binary := itestLndBinary if runtime.GOOS == "windows" { // Windows (even in a bash like environment like git bash as on diff --git a/lntest/neutrino.go b/lntest/neutrino.go index 25f2af24..d8e4596c 100644 --- a/lntest/neutrino.go +++ b/lntest/neutrino.go @@ -29,7 +29,7 @@ func (b NeutrinoBackendConfig) GenArgs() []string { // ConnectMiner is called to establish a connection to the test miner. func (b NeutrinoBackendConfig) ConnectMiner() error { - return fmt.Errorf("unimplemented") + return nil } // DisconnectMiner is called to disconnect the miner. From 65383a14940fcdda2692632a1c0904ee77e08fac Mon Sep 17 00:00:00 2001 From: yyforyongyu Date: Sun, 13 Sep 2020 11:25:46 +0800 Subject: [PATCH 4/4] build: update to btcd version with notfound msg fix --- go.mod | 2 +- go.sum | 2 ++ 2 files changed, 3 insertions(+), 1 deletion(-) diff --git a/go.mod b/go.mod index 4c0af208..af026089 100644 --- a/go.mod +++ b/go.mod @@ -5,7 +5,7 @@ require ( github.com/NebulousLabs/fastrand v0.0.0-20181203155948-6fb6489aac4e // indirect github.com/NebulousLabs/go-upnp v0.0.0-20180202185039-29b680b06c82 github.com/Yawning/aez v0.0.0-20180114000226-4dad034d9db2 - github.com/btcsuite/btcd v0.20.1-beta.0.20200730232343-1db1b6f8217f + github.com/btcsuite/btcd v0.20.1-beta.0.20200903105316-61634447e719 github.com/btcsuite/btclog v0.0.0-20170628155309-84c8d2346e9f github.com/btcsuite/btcutil v1.0.2 github.com/btcsuite/btcutil/psbt v1.0.2 diff --git a/go.sum b/go.sum index 3f2b8b91..940fef2c 100644 --- a/go.sum +++ b/go.sum @@ -29,6 +29,8 @@ github.com/btcsuite/btcd v0.20.1-beta/go.mod h1:wVuoA8VJLEcwgqHBwHmzLRazpKxTv13P github.com/btcsuite/btcd v0.20.1-beta.0.20200513120220-b470eee47728/go.mod h1:wVuoA8VJLEcwgqHBwHmzLRazpKxTv13Px/pDuV7OomQ= github.com/btcsuite/btcd v0.20.1-beta.0.20200730232343-1db1b6f8217f h1:m/GhMTvDQLbID616c4TYdHyt0MZ9lH5B/nf9Lu3okCY= github.com/btcsuite/btcd v0.20.1-beta.0.20200730232343-1db1b6f8217f/go.mod h1:ZSWyehm27aAuS9bvkATT+Xte3hjHZ+MRgMY/8NJ7K94= +github.com/btcsuite/btcd v0.20.1-beta.0.20200903105316-61634447e719 h1:EVCN2/T2EhbccMSuV3iM6cVcVVYSzmsx4EP3fWgdFGQ= +github.com/btcsuite/btcd v0.20.1-beta.0.20200903105316-61634447e719/go.mod h1:ZSWyehm27aAuS9bvkATT+Xte3hjHZ+MRgMY/8NJ7K94= github.com/btcsuite/btclog v0.0.0-20170628155309-84c8d2346e9f h1:bAs4lUbRJpnnkd9VhRV3jjAVU7DJVjMaK+IsvSeZvFo= github.com/btcsuite/btclog v0.0.0-20170628155309-84c8d2346e9f/go.mod h1:TdznJufoqS23FtqVCzL0ZqgP5MqXbb4fg/WgDys70nA= github.com/btcsuite/btcutil v0.0.0-20190425235716-9e5f4b9a998d h1:yJzD/yFppdVCf6ApMkVy8cUxV0XrxdP9rVf6D87/Mng=