From b4449ab55f1eb17c5b95c42b2fd0a202904af4c6 Mon Sep 17 00:00:00 2001 From: Oliver Gugger Date: Fri, 15 Mar 2024 12:54:29 +0100 Subject: [PATCH 01/13] GitHub: use bitcoind v26.0 for CI, remove default value To make it less confusing what version of bitcoind is actually installed, we now require the version to be specified as a command line argument. Because we tag the version in the docker image tag as bitcoin-core:XX but the binary internally is located under /opt/bitcoin-XX.Y/, we manually set Y to 0. --- .github/workflows/main.yml | 6 +++--- .travis.yml | 4 ++-- scripts/install_bitcoind.sh | 10 ++++++++-- 3 files changed, 13 insertions(+), 7 deletions(-) diff --git a/.github/workflows/main.yml b/.github/workflows/main.yml index 4a87854bf4..61fbeef589 100644 --- a/.github/workflows/main.yml +++ b/.github/workflows/main.yml @@ -21,7 +21,7 @@ defaults: shell: bash env: - BITCOIN_VERSION: "23.0" + BITCOIN_VERSION: "26" # If you change this value, please change it in the following files as well: # /.travis.yml @@ -215,7 +215,7 @@ jobs: key-prefix: unit-test - name: install bitcoind - run: ./scripts/install_bitcoind.sh + run: ./scripts/install_bitcoind.sh $BITCOIN_VERSION - name: run ${{ matrix.unit_type }} run: make log="stdlog debug" ${{ matrix.unit_type }} @@ -271,7 +271,7 @@ jobs: key-prefix: integration-test - name: install bitcoind - run: ./scripts/install_bitcoind.sh + run: ./scripts/install_bitcoind.sh $BITCOIN_VERSION - name: run ${{ matrix.name }} run: make itest-parallel ${{ matrix.args }} diff --git a/.travis.yml b/.travis.yml index 1d44fe7a68..b7c7734d94 100644 --- a/.travis.yml +++ b/.travis.yml @@ -26,7 +26,7 @@ go: env: global: - GOCACHE=$HOME/.go-build - - BITCOIN_VERSION="22.0" + - BITCOIN_VERSION="26" sudo: required @@ -35,7 +35,7 @@ jobs: - stage: Integration Test name: Bitcoind Integration ARM script: - - bash ./scripts/install_bitcoind.sh + - bash ./scripts/install_bitcoind.sh $BITCOIN_VERSION - GOMEMLIMIT=1024MiB GOARM=7 GOARCH=arm GOOS=linux travis_wait 120 make itest-parallel backend=bitcoind tranches=8 arch: arm64 diff --git a/scripts/install_bitcoind.sh b/scripts/install_bitcoind.sh index 7e70e4605f..5c655a63d4 100755 --- a/scripts/install_bitcoind.sh +++ b/scripts/install_bitcoind.sh @@ -2,9 +2,15 @@ set -ev -BITCOIND_VERSION=${BITCOIN_VERSION:-25.0} +BITCOIND_VERSION=$1 + +if [ -z "$BITCOIND_VERSION" ]; then + echo "Must specify a version of bitcoind to install." + echo "Usage: install_bitcoind.sh " + exit 1 +fi docker pull lightninglabs/bitcoin-core:$BITCOIND_VERSION CONTAINER_ID=$(docker create lightninglabs/bitcoin-core:$BITCOIND_VERSION) -sudo docker cp $CONTAINER_ID:/opt/bitcoin-$BITCOIND_VERSION/bin/bitcoind /usr/local/bin/bitcoind +sudo docker cp $CONTAINER_ID:/opt/bitcoin-$BITCOIND_VERSION.0/bin/bitcoind /usr/local/bin/bitcoind docker rm $CONTAINER_ID From 11ba14ab02005be5dd16bef23debf6115e6a85c2 Mon Sep 17 00:00:00 2001 From: Oliver Gugger Date: Fri, 15 Mar 2024 12:54:30 +0100 Subject: [PATCH 02/13] chainntnfs: add netParams arg to backend funcs --- chainntnfs/bitcoindnotify/bitcoind_test.go | 8 ++++---- chainntnfs/btcdnotify/btcd_test.go | 4 ++-- chainntnfs/test/test_interface.go | 10 ++++++---- chainntnfs/test_utils.go | 22 ++++++++++++---------- 4 files changed, 24 insertions(+), 20 deletions(-) diff --git a/chainntnfs/bitcoindnotify/bitcoind_test.go b/chainntnfs/bitcoindnotify/bitcoind_test.go index 3eefe8e328..03572ad0fe 100644 --- a/chainntnfs/bitcoindnotify/bitcoind_test.go +++ b/chainntnfs/bitcoindnotify/bitcoind_test.go @@ -120,11 +120,11 @@ func TestHistoricalConfDetailsTxIndex(t *testing.T) { func testHistoricalConfDetailsTxIndex(t *testing.T, rpcPolling bool) { miner := chainntnfs.NewMiner( - t, []string{"--txindex"}, true, 25, + t, chainntnfs.NetParams, []string{"--txindex"}, true, 25, ) bitcoindConn := chainntnfs.NewBitcoindBackend( - t, miner.P2PAddress(), true, rpcPolling, + t, chainntnfs.NetParams, miner.P2PAddress(), true, rpcPolling, ) hintCache := initHintCache(t) @@ -217,10 +217,10 @@ func TestHistoricalConfDetailsNoTxIndex(t *testing.T) { } func testHistoricalConfDetailsNoTxIndex(t *testing.T, rpcpolling bool) { - miner := chainntnfs.NewMiner(t, nil, true, 25) + miner := chainntnfs.NewMiner(t, chainntnfs.NetParams, nil, true, 25) bitcoindConn := chainntnfs.NewBitcoindBackend( - t, miner.P2PAddress(), false, rpcpolling, + t, chainntnfs.NetParams, miner.P2PAddress(), false, rpcpolling, ) hintCache := initHintCache(t) diff --git a/chainntnfs/btcdnotify/btcd_test.go b/chainntnfs/btcdnotify/btcd_test.go index 517710be56..240c5a24ce 100644 --- a/chainntnfs/btcdnotify/btcd_test.go +++ b/chainntnfs/btcdnotify/btcd_test.go @@ -74,7 +74,7 @@ func TestHistoricalConfDetailsTxIndex(t *testing.T) { t.Parallel() harness := chainntnfs.NewMiner( - t, []string{"--txindex"}, true, 25, + t, chainntnfs.NetParams, []string{"--txindex"}, true, 25, ) notifier := setUpNotifier(t, harness) @@ -145,7 +145,7 @@ func TestHistoricalConfDetailsTxIndex(t *testing.T) { func TestHistoricalConfDetailsNoTxIndex(t *testing.T) { t.Parallel() - harness := chainntnfs.NewMiner(t, nil, true, 25) + harness := chainntnfs.NewMiner(t, chainntnfs.NetParams, nil, true, 25) notifier := setUpNotifier(t, harness) diff --git a/chainntnfs/test/test_interface.go b/chainntnfs/test/test_interface.go index de01cc149a..a091c286e3 100644 --- a/chainntnfs/test/test_interface.go +++ b/chainntnfs/test/test_interface.go @@ -1905,7 +1905,7 @@ func TestInterfaces(t *testing.T, targetBackEnd string) { // dedicated miner to generate blocks, cause re-orgs, etc. We'll set up // this node with a chain length of 125, so we have plenty of BTC to // play around with. - miner := chainntnfs.NewMiner(t, nil, true, 25) + miner := chainntnfs.NewMiner(t, chainntnfs.NetParams, nil, true, 25) rpcConfig := miner.RPCConfig() p2pAddr := miner.P2PAddress() @@ -1945,7 +1945,7 @@ func TestInterfaces(t *testing.T, targetBackEnd string) { case "bitcoind": var bitcoindConn *chain.BitcoindConn bitcoindConn = chainntnfs.NewBitcoindBackend( - t, p2pAddr, true, false, + t, chainntnfs.NetParams, p2pAddr, true, false, ) newNotifier = func() (chainntnfs.TestChainNotifier, error) { return bitcoindnotify.New( @@ -1957,7 +1957,7 @@ func TestInterfaces(t *testing.T, targetBackEnd string) { case "bitcoind-rpc-polling": var bitcoindConn *chain.BitcoindConn bitcoindConn = chainntnfs.NewBitcoindBackend( - t, p2pAddr, true, true, + t, chainntnfs.NetParams, p2pAddr, true, true, ) newNotifier = func() (chainntnfs.TestChainNotifier, error) { return bitcoindnotify.New( @@ -1976,7 +1976,9 @@ func TestInterfaces(t *testing.T, targetBackEnd string) { case "neutrino": var spvNode *neutrino.ChainService - spvNode = chainntnfs.NewNeutrinoBackend(t, p2pAddr) + spvNode = chainntnfs.NewNeutrinoBackend( + t, chainntnfs.NetParams, p2pAddr, + ) newNotifier = func() (chainntnfs.TestChainNotifier, error) { return neutrinonotify.New( spvNode, hintCache, hintCache, diff --git a/chainntnfs/test_utils.go b/chainntnfs/test_utils.go index 8bd994f07d..73bd2cf69a 100644 --- a/chainntnfs/test_utils.go +++ b/chainntnfs/test_utils.go @@ -6,8 +6,8 @@ package chainntnfs import ( "errors" "fmt" - "io/ioutil" "math/rand" + "os" "os/exec" "path/filepath" "testing" @@ -166,8 +166,8 @@ func CreateSpendTx(t *testing.T, prevOutPoint *wire.OutPoint, // NewMiner spawns testing harness backed by a btcd node that can serve as a // miner. -func NewMiner(t *testing.T, extraArgs []string, createChain bool, - spendableOutputs uint32) *rpctest.Harness { +func NewMiner(t *testing.T, netParams *chaincfg.Params, extraArgs []string, + createChain bool, spendableOutputs uint32) *rpctest.Harness { t.Helper() @@ -175,7 +175,7 @@ func NewMiner(t *testing.T, extraArgs []string, createChain bool, trickle := fmt.Sprintf("--trickleinterval=%v", TrickleInterval) extraArgs = append(extraArgs, trickle) - node, err := rpctest.New(NetParams, nil, extraArgs, "") + node, err := rpctest.New(netParams, nil, extraArgs, "") require.NoError(t, err, "unable to create backend node") t.Cleanup(func() { require.NoError(t, node.TearDown()) @@ -194,15 +194,15 @@ func NewMiner(t *testing.T, extraArgs []string, createChain bool, // can be set to determine whether bitcoind's RPC polling interface should be // used for block and tx notifications or if its ZMQ interface should be used. // A connection to the newly spawned bitcoind node is returned. -func NewBitcoindBackend(t *testing.T, minerAddr string, txindex, - rpcpolling bool) *chain.BitcoindConn { +func NewBitcoindBackend(t *testing.T, netParams *chaincfg.Params, + minerAddr string, txindex, rpcpolling bool) *chain.BitcoindConn { t.Helper() // 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") + tempBitcoindDir, err := os.MkdirTemp("", "bitcoind") require.NoError(t, err, "unable to create temp dir") rpcPort := rand.Intn(65536-1024) + 1024 @@ -236,7 +236,7 @@ func NewBitcoindBackend(t *testing.T, minerAddr string, txindex, // Wait for the bitcoind instance to start up. host := fmt.Sprintf("127.0.0.1:%d", rpcPort) cfg := &chain.BitcoindConfig{ - ChainParams: NetParams, + ChainParams: netParams, Host: host, User: "weks", Pass: "weks", @@ -279,7 +279,9 @@ func NewBitcoindBackend(t *testing.T, minerAddr string, txindex, // NewNeutrinoBackend spawns a new neutrino node that connects to a miner at // the specified address. -func NewNeutrinoBackend(t *testing.T, minerAddr string) *neutrino.ChainService { +func NewNeutrinoBackend(t *testing.T, netParams *chaincfg.Params, + minerAddr string) *neutrino.ChainService { + t.Helper() spvDir := t.TempDir() @@ -300,7 +302,7 @@ func NewNeutrinoBackend(t *testing.T, minerAddr string) *neutrino.ChainService { spvConfig := neutrino.Config{ DataDir: spvDir, Database: spvDatabase, - ChainParams: *NetParams, + ChainParams: *netParams, ConnectPeers: []string{minerAddr}, } spvNode, err := neutrino.NewChainService(spvConfig) From 9cd7285439928a65b4ee8009ca5795a1946cd8e6 Mon Sep 17 00:00:00 2001 From: Oliver Gugger Date: Fri, 15 Mar 2024 12:54:31 +0100 Subject: [PATCH 03/13] itest+lntest: use system wide unique ports everywhere With this commit we create a new function that returns system wide unique ports by using a single file to keep track of previously used ports. We'll want to use this everywhere whenever we need to listen on a new, random port during unit or integration tests. Because we now have a unique source, we don't need to apply the port offset that was used for the different tranches of parallel running integration tests before. --- itest/lnd_etcd_failover_test.go | 6 +- itest/lnd_network_test.go | 5 +- itest/lnd_test.go | 7 +- itest/lnd_watchtower_test.go | 3 +- lntest/bitcoind_common.go | 10 +-- lntest/fee_service.go | 6 +- lntest/node/config.go | 71 +++------------ lntest/port/port.go | 150 ++++++++++++++++++++++++++++++++ 8 files changed, 184 insertions(+), 74 deletions(-) create mode 100644 lntest/port/port.go diff --git a/itest/lnd_etcd_failover_test.go b/itest/lnd_etcd_failover_test.go index 9ec6bd8a81..c2a345889c 100644 --- a/itest/lnd_etcd_failover_test.go +++ b/itest/lnd_etcd_failover_test.go @@ -13,7 +13,7 @@ import ( "github.com/lightningnetwork/lnd/lncfg" "github.com/lightningnetwork/lnd/lnrpc" "github.com/lightningnetwork/lnd/lntest" - "github.com/lightningnetwork/lnd/lntest/node" + "github.com/lightningnetwork/lnd/lntest/port" "github.com/stretchr/testify/require" ) @@ -56,8 +56,8 @@ func testEtcdFailover(ht *lntest.HarnessTest) { func testEtcdFailoverCase(ht *lntest.HarnessTest, kill bool) { etcdCfg, cleanup, err := kvdb.StartEtcdTestBackend( - ht.T.TempDir(), uint16(node.NextAvailablePort()), - uint16(node.NextAvailablePort()), "", + ht.T.TempDir(), uint16(port.NextAvailablePort()), + uint16(port.NextAvailablePort()), "", ) require.NoError(ht, err, "Failed to start etcd instance") defer cleanup() diff --git a/itest/lnd_network_test.go b/itest/lnd_network_test.go index b4c73af60a..fd17d04657 100644 --- a/itest/lnd_network_test.go +++ b/itest/lnd_network_test.go @@ -9,6 +9,7 @@ import ( "github.com/lightningnetwork/lnd/lnrpc" "github.com/lightningnetwork/lnd/lntest" "github.com/lightningnetwork/lnd/lntest/node" + "github.com/lightningnetwork/lnd/lntest/port" "github.com/lightningnetwork/lnd/lntest/wait" "github.com/stretchr/testify/require" ) @@ -111,7 +112,7 @@ func testReconnectAfterIPChange(ht *lntest.HarnessTest) { // We derive an extra port for Dave, and we initialise his node with // the port advertised as `--externalip` arguments. - ip2 := node.NextAvailablePort() + ip2 := port.NextAvailablePort() // Create a new node, Dave, which will initialize a P2P port for him. daveArgs := []string{fmt.Sprintf("--externalip=127.0.0.1:%d", ip2)} @@ -190,7 +191,7 @@ func testReconnectAfterIPChange(ht *lntest.HarnessTest) { // address. // Change Dave's listening port and restart. - dave.Cfg.P2PPort = node.NextAvailablePort() + dave.Cfg.P2PPort = port.NextAvailablePort() dave.Cfg.ExtraArgs = []string{ fmt.Sprintf( "--externalip=127.0.0.1:%d", dave.Cfg.P2PPort, diff --git a/itest/lnd_test.go b/itest/lnd_test.go index 9c823b5a84..b5886bc299 100644 --- a/itest/lnd_test.go +++ b/itest/lnd_test.go @@ -17,6 +17,7 @@ import ( "github.com/lightningnetwork/lnd/lnrpc" "github.com/lightningnetwork/lnd/lntest" "github.com/lightningnetwork/lnd/lntest/node" + "github.com/lightningnetwork/lnd/lntest/port" "github.com/lightningnetwork/lnd/lntest/wait" "github.com/stretchr/testify/require" "google.golang.org/grpc/grpclog" @@ -90,7 +91,6 @@ func TestLightningNetworkDaemon(t *testing.T) { // Get the test cases to be run in this tranche. testCases, trancheIndex, trancheOffset := getTestCaseSplitTranche() - node.ApplyPortOffset(uint32(trancheIndex) * 1000) // Create a simple fee service. feeService := lntest.NewFeeService(t) @@ -216,9 +216,10 @@ func init() { // Before we start any node, we need to make sure that any btcd node // that is started through the RPC harness uses a unique port as well // to avoid any port collisions. - rpctest.ListenAddressGenerator = node.GenerateBtcdListenerAddresses + rpctest.ListenAddressGenerator = + port.GenerateSystemUniqueListenerAddresses - // Swap out grpc's default logger with out fake logger which drops the + // Swap out grpc's default logger with our fake logger which drops the // statements on the floor. fakeLogger := grpclog.NewLoggerV2(io.Discard, io.Discard, io.Discard) grpclog.SetLoggerV2(fakeLogger) diff --git a/itest/lnd_watchtower_test.go b/itest/lnd_watchtower_test.go index bfd1425a59..ad724e45ad 100644 --- a/itest/lnd_watchtower_test.go +++ b/itest/lnd_watchtower_test.go @@ -13,6 +13,7 @@ import ( "github.com/lightningnetwork/lnd/lnrpc/wtclientrpc" "github.com/lightningnetwork/lnd/lntest" "github.com/lightningnetwork/lnd/lntest/node" + "github.com/lightningnetwork/lnd/lntest/port" "github.com/lightningnetwork/lnd/lntest/rpc" "github.com/lightningnetwork/lnd/lntest/wait" "github.com/stretchr/testify/require" @@ -595,7 +596,7 @@ func testRevokedCloseRetributionAltruistWatchtowerCase(ht *lntest.HarnessTest, func setUpNewTower(ht *lntest.HarnessTest, name, externalIP string) ([]byte, string, *rpc.HarnessRPC) { - port := node.NextAvailablePort() + port := port.NextAvailablePort() listenAddr := fmt.Sprintf("0.0.0.0:%d", port) diff --git a/lntest/bitcoind_common.go b/lntest/bitcoind_common.go index 9ce962dc26..b5c43c4481 100644 --- a/lntest/bitcoind_common.go +++ b/lntest/bitcoind_common.go @@ -15,6 +15,7 @@ import ( "github.com/btcsuite/btcd/chaincfg" "github.com/btcsuite/btcd/rpcclient" "github.com/lightningnetwork/lnd/lntest/node" + "github.com/lightningnetwork/lnd/lntest/port" ) // logDirPattern is the pattern of the name of the temporary log directory. @@ -111,11 +112,10 @@ func newBackend(miner string, netParams *chaincfg.Params, extraArgs []string, } zmqBlockAddr := fmt.Sprintf("tcp://127.0.0.1:%d", - node.NextAvailablePort()) - zmqTxAddr := fmt.Sprintf("tcp://127.0.0.1:%d", - node.NextAvailablePort()) - rpcPort := node.NextAvailablePort() - p2pPort := node.NextAvailablePort() + port.NextAvailablePort()) + zmqTxAddr := fmt.Sprintf("tcp://127.0.0.1:%d", port.NextAvailablePort()) + rpcPort := port.NextAvailablePort() + p2pPort := port.NextAvailablePort() cmdArgs := []string{ "-datadir=" + tempBitcoindDir, diff --git a/lntest/fee_service.go b/lntest/fee_service.go index 592430b8e8..49bd953ac2 100644 --- a/lntest/fee_service.go +++ b/lntest/fee_service.go @@ -10,7 +10,7 @@ import ( "testing" "github.com/lightningnetwork/lnd" - "github.com/lightningnetwork/lnd/lntest/node" + "github.com/lightningnetwork/lnd/lntest/port" "github.com/lightningnetwork/lnd/lnwallet/chainfee" "github.com/stretchr/testify/require" ) @@ -60,11 +60,11 @@ type FeeService struct { // Compile-time check for the WebFeeService interface. var _ WebFeeService = (*FeeService)(nil) -// Start spins up a go-routine to serve fee estimates. +// NewFeeService spins up a go-routine to serve fee estimates. func NewFeeService(t *testing.T) *FeeService { t.Helper() - port := node.NextAvailablePort() + port := port.NextAvailablePort() f := FeeService{ T: t, url: fmt.Sprintf( diff --git a/lntest/node/config.go b/lntest/node/config.go index 1573af42e0..5a7013a215 100644 --- a/lntest/node/config.go +++ b/lntest/node/config.go @@ -4,16 +4,16 @@ import ( "flag" "fmt" "io" - "net" "os" "path" "path/filepath" - "sync/atomic" "github.com/btcsuite/btcd/chaincfg" + "github.com/btcsuite/btcd/integration/rpctest" "github.com/lightningnetwork/lnd" "github.com/lightningnetwork/lnd/chanbackup" "github.com/lightningnetwork/lnd/kvdb/etcd" + "github.com/lightningnetwork/lnd/lntest/port" "github.com/lightningnetwork/lnd/lntest/wait" ) @@ -25,18 +25,9 @@ const ( // DefaultCSV is the CSV delay (remotedelay) we will start our test // nodes with. DefaultCSV = 4 - - // defaultNodePort is the start of the range for listening ports of - // harness nodes. Ports are monotonically increasing starting from this - // number and are determined by the results of NextAvailablePort(). - defaultNodePort = 5555 ) var ( - // lastPort is the last port determined to be free for use by a new - // node. It should be used atomically. - lastPort uint32 = defaultNodePort - // logOutput is a flag that can be set to append the output from the // seed nodes to log files. logOutput = flag.Bool("logoutput", false, @@ -171,16 +162,16 @@ func (cfg BaseNodeConfig) ChanBackupPath() string { // current lightning network test. func (cfg *BaseNodeConfig) GenerateListeningPorts() { if cfg.P2PPort == 0 { - cfg.P2PPort = NextAvailablePort() + cfg.P2PPort = port.NextAvailablePort() } if cfg.RPCPort == 0 { - cfg.RPCPort = NextAvailablePort() + cfg.RPCPort = port.NextAvailablePort() } if cfg.RESTPort == 0 { - cfg.RESTPort = NextAvailablePort() + cfg.RESTPort = port.NextAvailablePort() } if cfg.ProfilePort == 0 { - cfg.ProfilePort = NextAvailablePort() + cfg.ProfilePort = port.NextAvailablePort() } } @@ -259,13 +250,13 @@ func (cfg *BaseNodeConfig) GenArgs() []string { args = append( args, fmt.Sprintf( "--db.etcd.embedded_client_port=%v", - NextAvailablePort(), + port.NextAvailablePort(), ), ) args = append( args, fmt.Sprintf( "--db.etcd.embedded_peer_port=%v", - NextAvailablePort(), + port.NextAvailablePort(), ), ) args = append( @@ -333,34 +324,6 @@ func ExtraArgsEtcd(etcdCfg *etcd.Config, name string, cluster bool, return extraArgs } -// NextAvailablePort returns the first port that is available for listening by -// a new node. It panics if no port is found and the maximum available TCP port -// is reached. -func NextAvailablePort() int { - port := atomic.AddUint32(&lastPort, 1) - for port < 65535 { - // If there are no errors while attempting to listen on this - // port, close the socket and return it as available. While it - // could be the case that some other process picks up this port - // between the time the socket is closed and it's reopened in - // the harness node, in practice in CI servers this seems much - // less likely than simply some other process already being - // bound at the start of the tests. - addr := fmt.Sprintf(ListenerFormat, port) - l, err := net.Listen("tcp4", addr) - if err == nil { - err := l.Close() - if err == nil { - return int(port) - } - } - port = atomic.AddUint32(&lastPort, 1) - } - - // No ports available? Must be a mistake. - panic("no ports available for listening") -} - // GetLogDir returns the passed --logdir flag or the default value if it wasn't // set. func GetLogDir() string { @@ -402,16 +365,10 @@ func GetBtcdBinary() string { return "" } -// GenerateBtcdListenerAddresses is a function that returns two listener -// addresses with unique ports and should be used to overwrite rpctest's -// default generator which is prone to use colliding ports. -func GenerateBtcdListenerAddresses() (string, string) { - return fmt.Sprintf(ListenerFormat, NextAvailablePort()), - fmt.Sprintf(ListenerFormat, NextAvailablePort()) -} - -// ApplyPortOffset adds the given offset to the lastPort variable, making it -// possible to run the tests in parallel without colliding on the same ports. -func ApplyPortOffset(offset uint32) { - _ = atomic.AddUint32(&lastPort, offset) +func init() { + // Before we start any node, we need to make sure that any btcd or + // bitcoind node that is started through the RPC harness uses a unique + // port as well to avoid any port collisions. + rpctest.ListenAddressGenerator = + port.GenerateSystemUniqueListenerAddresses } diff --git a/lntest/port/port.go b/lntest/port/port.go new file mode 100644 index 0000000000..28db1918dc --- /dev/null +++ b/lntest/port/port.go @@ -0,0 +1,150 @@ +package port + +import ( + "fmt" + "net" + "os" + "path/filepath" + "strconv" + "sync" + "time" +) + +const ( + // ListenerFormat is the format string that is used to generate local + // listener addresses. + ListenerFormat = "127.0.0.1:%d" + + // defaultNodePort is the start of the range for listening ports of + // harness nodes. Ports are monotonically increasing starting from this + // number and are determined by the results of NextAvailablePort(). + defaultNodePort int = 10000 + + // uniquePortFile is the name of the file that is used to store the + // last port that was used by a node. This is used to make sure that + // the same port is not used by multiple nodes at the same time. The + // file is located in the temp directory of a system. + uniquePortFile = "rpctest-port" +) + +var ( + // portFileMutex is a mutex that is used to make sure that the port file + // is not accessed by multiple goroutines of the same process at the + // same time. This is used in conjunction with the lock file to make + // sure that the port file is not accessed by multiple processes at the + // same time either. So the lock file is to guard between processes and + // the mutex is to guard between goroutines of the same process. + portFileMutex sync.Mutex +) + +// NextAvailablePort returns the first port that is available for listening by a +// new node, using a lock file to make sure concurrent access for parallel tasks +// on the same system don't re-use the same port. +func NextAvailablePort() int { + portFileMutex.Lock() + defer portFileMutex.Unlock() + + lockFile := filepath.Join(os.TempDir(), uniquePortFile+".lock") + timeout := time.After(time.Second) + + var ( + lockFileHandle *os.File + err error + ) + for { + // Attempt to acquire the lock file. If it already exists, wait + // for a bit and retry. + lockFileHandle, err = os.OpenFile( + lockFile, os.O_CREATE|os.O_EXCL, 0600, + ) + if err == nil { + // Lock acquired. + break + } + + // Wait for a bit and retry. + select { + case <-timeout: + panic("timeout waiting for lock file") + case <-time.After(10 * time.Millisecond): + } + } + + // Release the lock file when we're done. + defer func() { + // Always close file first, Windows won't allow us to remove it + // otherwise. + _ = lockFileHandle.Close() + err := os.Remove(lockFile) + if err != nil { + panic(fmt.Errorf("couldn't remove lock file: %w", err)) + } + }() + + portFile := filepath.Join(os.TempDir(), uniquePortFile) + port, err := os.ReadFile(portFile) + if err != nil { + if !os.IsNotExist(err) { + panic(fmt.Errorf("error reading port file: %w", err)) + } + port = []byte(strconv.Itoa(defaultNodePort)) + } + + lastPort, err := strconv.Atoi(string(port)) + if err != nil { + panic(fmt.Errorf("error parsing port: %w", err)) + } + + // We take the next one. + lastPort++ + for lastPort < 65535 { + // If there are no errors while attempting to listen on this + // port, close the socket and return it as available. While it + // could be the case that some other process picks up this port + // between the time the socket is closed, and it's reopened in + // the harness node, in practice in CI servers this seems much + // less likely than simply some other process already being + // bound at the start of the tests. + addr := fmt.Sprintf(ListenerFormat, lastPort) + l, err := net.Listen("tcp4", addr) + if err == nil { + err := l.Close() + if err == nil { + err := os.WriteFile( + portFile, + []byte(strconv.Itoa(lastPort)), 0600, + ) + if err != nil { + panic(fmt.Errorf("error updating "+ + "port file: %w", err)) + } + + return lastPort + } + } + lastPort++ + + // Start from the beginning if we reached the end of the port + // range. We need to do this because the lock file now is + // persistent across runs on the same machine during the same + // boot/uptime cycle. So in order to make this work on + // developer's machines, we need to reset the port to the + // default value when we reach the end of the range. + if lastPort == 65535 { + lastPort = defaultNodePort + } + } + + // No ports available? Must be a mistake. + panic("no ports available for listening") +} + +// GenerateSystemUniqueListenerAddresses is a function that returns two +// listener addresses with unique ports per system and should be used to +// overwrite rpctest's default generator which is prone to use colliding ports. +func GenerateSystemUniqueListenerAddresses() (string, string) { + port1 := NextAvailablePort() + port2 := NextAvailablePort() + return fmt.Sprintf(ListenerFormat, port1), + fmt.Sprintf(ListenerFormat, port2) +} From 2cc28baa847178a90099fa2a242de1323a157cdb Mon Sep 17 00:00:00 2001 From: Oliver Gugger Date: Fri, 15 Mar 2024 12:54:33 +0100 Subject: [PATCH 04/13] chainntnfs: re-try miner connection more often Make sure we wait for the initial connection long enough. --- chainntnfs/test_utils.go | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/chainntnfs/test_utils.go b/chainntnfs/test_utils.go index 73bd2cf69a..7c4e360d45 100644 --- a/chainntnfs/test_utils.go +++ b/chainntnfs/test_utils.go @@ -181,6 +181,14 @@ func NewMiner(t *testing.T, netParams *chaincfg.Params, extraArgs []string, require.NoError(t, node.TearDown()) }) + // We want to overwrite some of the connection settings to make the + // tests more robust. We might need to restart the backend while there + // are already blocks present, which will take a bit longer than the + // 1 second the default settings amount to. Doubling both values will + // give us retries up to 4 seconds. + node.MaxConnRetries = rpctest.DefaultMaxConnectionRetries * 2 + node.ConnectionRetryTimeout = rpctest.DefaultConnectionRetryTimeout * 2 + if err := node.SetUp(createChain, spendableOutputs); err != nil { t.Fatalf("unable to set up backend node: %v", err) } From 2884389ce4cade588aba1b506429b73a7a8c964d Mon Sep 17 00:00:00 2001 From: Oliver Gugger Date: Fri, 15 Mar 2024 12:54:34 +0100 Subject: [PATCH 05/13] chainntnfs: fix issues with NewBitcoindBackend This commit fixes a number of issues with our temporary bitcoind nodes that we spin up for unit tests: - We didn't remove the node's temporary data dir after tests finished. - We used random ports which lead to unwanted port collisions. - We used ipc:// unix sockets for ZMQ which currently isn't supported in bitcoind v26.0 due to a regression. Since we can reliably create new non-colliding ports now, we should use TCP for ZMQ anyway. --- chainntnfs/test_utils.go | 22 ++++++++++------------ 1 file changed, 10 insertions(+), 12 deletions(-) diff --git a/chainntnfs/test_utils.go b/chainntnfs/test_utils.go index 7c4e360d45..a0be772cfe 100644 --- a/chainntnfs/test_utils.go +++ b/chainntnfs/test_utils.go @@ -6,8 +6,6 @@ package chainntnfs import ( "errors" "fmt" - "math/rand" - "os" "os/exec" "path/filepath" "testing" @@ -25,6 +23,7 @@ import ( "github.com/btcsuite/btcwallet/walletdb" "github.com/lightninglabs/neutrino" "github.com/lightningnetwork/lnd/kvdb" + "github.com/lightningnetwork/lnd/lntest/port" "github.com/lightningnetwork/lnd/lntest/wait" "github.com/stretchr/testify/require" ) @@ -207,15 +206,13 @@ func NewBitcoindBackend(t *testing.T, netParams *chaincfg.Params, t.Helper() - // 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 := os.MkdirTemp("", "bitcoind") - require.NoError(t, err, "unable to create temp dir") + tempBitcoindDir := t.TempDir() - rpcPort := rand.Intn(65536-1024) + 1024 - zmqBlockHost := "ipc:///" + tempBitcoindDir + "/blocks.socket" - zmqTxHost := "ipc:///" + tempBitcoindDir + "/tx.socket" + rpcPort := port.NextAvailablePort() + zmqBlockPort := port.NextAvailablePort() + zmqTxPort := port.NextAvailablePort() + zmqBlockHost := fmt.Sprintf("tcp://127.0.0.1:%d", zmqBlockPort) + zmqTxHost := fmt.Sprintf("tcp://127.0.0.1:%d", zmqTxPort) args := []string{ "-connect=" + minerAddr, @@ -268,7 +265,7 @@ func NewBitcoindBackend(t *testing.T, netParams *chaincfg.Params, } 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 { @@ -278,7 +275,8 @@ func NewBitcoindBackend(t *testing.T, netParams *chaincfg.Params, return conn.Start() }, 10*time.Second) if err != nil { - t.Fatalf("unable to establish connection to bitcoind: %v", err) + t.Fatalf("unable to establish connection to bitcoind at %v: "+ + "%v", tempBitcoindDir, err) } t.Cleanup(conn.Stop) From c170a9830bbf854380f9e58421fdfbd478b76927 Mon Sep 17 00:00:00 2001 From: Oliver Gugger Date: Fri, 15 Mar 2024 12:54:35 +0100 Subject: [PATCH 06/13] multi: use chainntnfs.NewMiner for miners in unit tests With the chainntnfs.NewMiner now being optimized for not creating nodes with colliding ports, we use it in all unit tests that spin up temporary miners. --- chainntnfs/test/test_interface.go | 27 ++++++--------------------- lnwallet/btcwallet/signer_test.go | 18 ++++++++---------- lnwallet/test/test_interface.go | 17 +++++------------ routing/chainview/interface_test.go | 29 +++++++---------------------- 4 files changed, 26 insertions(+), 65 deletions(-) diff --git a/chainntnfs/test/test_interface.go b/chainntnfs/test/test_interface.go index a091c286e3..a11f751c68 100644 --- a/chainntnfs/test/test_interface.go +++ b/chainntnfs/test/test_interface.go @@ -1043,14 +1043,9 @@ func testReorgConf(miner *rpctest.Harness, notifier chainntnfs.TestChainNotifier, scriptDispatch bool, t *testing.T) { // Set up a new miner that we can use to cause a reorg. - miner2, err := rpctest.New( - chainntnfs.NetParams, nil, []string{"--txindex"}, "", + miner2 := chainntnfs.NewMiner( + t, chainntnfs.NetParams, []string{"--txindex"}, false, 0, ) - require.NoError(t, err, "unable to create mining node") - if err := miner2.SetUp(false, 0); err != nil { - t.Fatalf("unable to set up mining node: %v", err) - } - defer miner2.TearDown() // We start by connecting the new miner to our original miner, // such that it will sync to our original chain. @@ -1204,14 +1199,9 @@ func testReorgSpend(miner *rpctest.Harness, require.NoError(t, err, "unable to register for spend") // Set up a new miner that we can use to cause a reorg. - miner2, err := rpctest.New( - chainntnfs.NetParams, nil, []string{"--txindex"}, "", + miner2 := chainntnfs.NewMiner( + t, chainntnfs.NetParams, []string{"--txindex"}, false, 0, ) - require.NoError(t, err, "unable to create mining node") - if err := miner2.SetUp(false, 0); err != nil { - t.Fatalf("unable to set up mining node: %v", err) - } - defer miner2.TearDown() // We start by connecting the new miner to our original miner, in order // to have a consistent view of the chain from both miners. They should @@ -1527,14 +1517,9 @@ func testCatchUpOnMissedBlocksWithReorg(miner1 *rpctest.Harness, var wg sync.WaitGroup // Set up a new miner that we can use to cause a reorg. - miner2, err := rpctest.New( - chainntnfs.NetParams, nil, []string{"--txindex"}, "", + miner2 := chainntnfs.NewMiner( + t, chainntnfs.NetParams, []string{"--txindex"}, false, 0, ) - require.NoError(t, err, "unable to create mining node") - if err := miner2.SetUp(false, 0); err != nil { - t.Fatalf("unable to set up mining node: %v", err) - } - defer miner2.TearDown() // We start by connecting the new miner to our original miner, // such that it will sync to our original chain. diff --git a/lnwallet/btcwallet/signer_test.go b/lnwallet/btcwallet/signer_test.go index d51714ad01..7e5a3a20ee 100644 --- a/lnwallet/btcwallet/signer_test.go +++ b/lnwallet/btcwallet/signer_test.go @@ -18,6 +18,7 @@ import ( "github.com/btcsuite/btcwallet/chain" "github.com/btcsuite/btcwallet/waddrmgr" "github.com/lightningnetwork/lnd/blockcache" + "github.com/lightningnetwork/lnd/chainntnfs" "github.com/lightningnetwork/lnd/input" "github.com/lightningnetwork/lnd/lnwallet" "github.com/stretchr/testify/require" @@ -294,8 +295,7 @@ func TestScriptImport(t *testing.T) { func newTestWallet(t *testing.T, netParams *chaincfg.Params, seedBytes []byte) (*BtcWallet, *rpctest.Harness) { - chainBackend, miner, backendCleanup := getChainBackend(t, netParams) - t.Cleanup(backendCleanup) + chainBackend, miner := getChainBackend(t, netParams) loaderOpt := LoaderWithLocalWalletDB(t.TempDir(), false, time.Minute) config := Config{ @@ -324,16 +324,16 @@ func newTestWallet(t *testing.T, netParams *chaincfg.Params, // getChainBackend returns a simple btcd based chain backend to back the wallet. func getChainBackend(t *testing.T, netParams *chaincfg.Params) (chain.Interface, - *rpctest.Harness, func()) { + *rpctest.Harness) { - miningNode, err := rpctest.New(netParams, nil, nil, "") - require.NoError(t, err) - require.NoError(t, miningNode.SetUp(true, 25)) + miningNode := chainntnfs.NewMiner( + t, netParams, []string{"--txindex"}, true, 25, + ) // Next, mine enough blocks in order for SegWit and the CSV package // soft-fork to activate on RegNet. numBlocks := netParams.MinerConfirmationWindow * 2 - _, err = miningNode.Client.Generate(numBlocks) + _, err := miningNode.Client.Generate(numBlocks) require.NoError(t, err) rpcConfig := miningNode.RPCConfig() @@ -343,9 +343,7 @@ func getChainBackend(t *testing.T, netParams *chaincfg.Params) (chain.Interface, ) require.NoError(t, err) - return chainClient, miningNode, func() { - _ = miningNode.TearDown() - } + return chainClient, miningNode } // hardenedKey returns a key of a hardened derivation key path. diff --git a/lnwallet/test/test_interface.go b/lnwallet/test/test_interface.go index caa222cf47..42f63be51f 100644 --- a/lnwallet/test/test_interface.go +++ b/lnwallet/test/test_interface.go @@ -2120,11 +2120,9 @@ func testReorgWalletBalance(r *rpctest.Harness, w *lnwallet.LightningWallet, // Now we cause a reorganization as follows. // Step 1: create a new miner and start it. - r2, err := rpctest.New(r.ActiveNet, nil, []string{"--txindex"}, "") - require.NoError(t, err, "unable to create mining node") - err = r2.SetUp(false, 0) - require.NoError(t, err, "unable to set up mining node") - defer r2.TearDown() + r2 := chainntnfs.NewMiner( + t, r.ActiveNet, []string{"--txindex"}, false, 0, + ) newBalance, err := w.ConfirmedBalance(1, lnwallet.DefaultAccountName) require.NoError(t, err, "unable to query for balance") if origBalance != newBalance { @@ -3103,14 +3101,9 @@ func TestLightningWallet(t *testing.T, targetBackEnd string) { // dedicated miner to generate blocks, cause re-orgs, etc. We'll set // up this node with a chain length of 125, so we have plenty of BTC // to play around with. - miningNode, err := rpctest.New( - netParams, nil, []string{"--txindex"}, "", + miningNode := chainntnfs.NewMiner( + t, netParams, []string{"--txindex"}, true, 25, ) - require.NoError(t, err, "unable to create mining node") - defer miningNode.TearDown() - if err := miningNode.SetUp(true, 25); err != nil { - t.Fatalf("unable to set up mining node: %v", err) - } // Next mine enough blocks in order for segwit and the CSV package // soft-fork to activate on RegNet. diff --git a/routing/chainview/interface_test.go b/routing/chainview/interface_test.go index 010f33807c..c3bf9ac063 100644 --- a/routing/chainview/interface_test.go +++ b/routing/chainview/interface_test.go @@ -25,6 +25,7 @@ import ( _ "github.com/btcsuite/btcwallet/walletdb/bdb" // Required to register the boltdb walletdb implementation. "github.com/lightninglabs/neutrino" "github.com/lightningnetwork/lnd/blockcache" + "github.com/lightningnetwork/lnd/chainntnfs" "github.com/lightningnetwork/lnd/channeldb" "github.com/lightningnetwork/lnd/kvdb" "github.com/lightningnetwork/lnd/lntest/wait" @@ -481,22 +482,9 @@ func testFilterBlockDisconnected(node *rpctest.Harness, // Create a node that has a shorter chain than the main chain, so we // can trigger a reorg. - reorgNode, err := rpctest.New(netParams, nil, []string{"--txindex"}, "") - require.NoError(t, err, "unable to create mining node") - defer reorgNode.TearDown() - - // We want to overwrite some of the connection settings to make the - // tests more robust. We might need to restart the backend while there - // are already blocks present, which will take a bit longer than the - // 1 second the default settings amount to. Doubling both values will - // give us retries up to 4 seconds. - reorgNode.MaxConnRetries = rpctest.DefaultMaxConnectionRetries * 2 - reorgNode.ConnectionRetryTimeout = rpctest.DefaultConnectionRetryTimeout * 2 - - // This node's chain will be 105 blocks. - if err := reorgNode.SetUp(true, 5); err != nil { - t.Fatalf("unable to set up mining node: %v", err) - } + reorgNode := chainntnfs.NewMiner( + t, netParams, []string{"--txindex"}, true, 5, + ) _, bestHeight, err := reorgNode.Client.GetBestBlock() require.NoError(t, err, "error getting best block") @@ -996,12 +984,9 @@ func TestFilteredChainView(t *testing.T) { // dedicated miner to generate blocks, cause re-orgs, etc. We'll set up // this node with a chain length of 125, so we have plenty of BTC to // play around with. - miner, err := rpctest.New(netParams, nil, []string{"--txindex"}, "") - require.NoError(t, err, "unable to create mining node") - defer miner.TearDown() - if err := miner.SetUp(true, 25); err != nil { - t.Fatalf("unable to set up mining node: %v", err) - } + miner := chainntnfs.NewMiner( + t, netParams, []string{"--txindex"}, true, 25, + ) rpcConfig := miner.RPCConfig() p2pAddr := miner.P2PAddress() From d40312c36be8054a20dc408269d6851dc2e86723 Mon Sep 17 00:00:00 2001 From: Oliver Gugger Date: Fri, 15 Mar 2024 12:54:36 +0100 Subject: [PATCH 07/13] multi: move unit test backend funcs to new package To avoid circular dependency issues between packages, we move the unit test backend creation function to a new package in the lntest parent package. --- chainntnfs/bitcoindnotify/bitcoind_test.go | 17 +- chainntnfs/btcdnotify/btcd_test.go | 9 +- chainntnfs/test/test_interface.go | 33 ++-- chainntnfs/test_utils.go | 184 +----------------- lntest/unittest/backend.go | 207 +++++++++++++++++++++ lnwallet/btcwallet/signer_test.go | 4 +- lnwallet/test/test_interface.go | 5 +- routing/chainview/interface_test.go | 6 +- 8 files changed, 250 insertions(+), 215 deletions(-) create mode 100644 lntest/unittest/backend.go diff --git a/chainntnfs/bitcoindnotify/bitcoind_test.go b/chainntnfs/bitcoindnotify/bitcoind_test.go index 03572ad0fe..8d54439a6c 100644 --- a/chainntnfs/bitcoindnotify/bitcoind_test.go +++ b/chainntnfs/bitcoindnotify/bitcoind_test.go @@ -15,6 +15,7 @@ import ( "github.com/lightningnetwork/lnd/blockcache" "github.com/lightningnetwork/lnd/chainntnfs" "github.com/lightningnetwork/lnd/channeldb" + "github.com/lightningnetwork/lnd/lntest/unittest" "github.com/lightningnetwork/lnd/lntest/wait" "github.com/stretchr/testify/require" ) @@ -61,7 +62,7 @@ func setUpNotifier(t *testing.T, bitcoindConn *chain.BitcoindConn, t.Helper() notifier := New( - bitcoindConn, chainntnfs.NetParams, spendHintCache, + bitcoindConn, unittest.NetParams, spendHintCache, confirmHintCache, blockCache, ) if err := notifier.Start(); err != nil { @@ -119,12 +120,12 @@ func TestHistoricalConfDetailsTxIndex(t *testing.T) { } func testHistoricalConfDetailsTxIndex(t *testing.T, rpcPolling bool) { - miner := chainntnfs.NewMiner( - t, chainntnfs.NetParams, []string{"--txindex"}, true, 25, + miner := unittest.NewMiner( + t, unittest.NetParams, []string{"--txindex"}, true, 25, ) - bitcoindConn := chainntnfs.NewBitcoindBackend( - t, chainntnfs.NetParams, miner.P2PAddress(), true, rpcPolling, + bitcoindConn := unittest.NewBitcoindBackend( + t, unittest.NetParams, miner.P2PAddress(), true, rpcPolling, ) hintCache := initHintCache(t) @@ -217,10 +218,10 @@ func TestHistoricalConfDetailsNoTxIndex(t *testing.T) { } func testHistoricalConfDetailsNoTxIndex(t *testing.T, rpcpolling bool) { - miner := chainntnfs.NewMiner(t, chainntnfs.NetParams, nil, true, 25) + miner := unittest.NewMiner(t, unittest.NetParams, nil, true, 25) - bitcoindConn := chainntnfs.NewBitcoindBackend( - t, chainntnfs.NetParams, miner.P2PAddress(), false, rpcpolling, + bitcoindConn := unittest.NewBitcoindBackend( + t, unittest.NetParams, miner.P2PAddress(), false, rpcpolling, ) hintCache := initHintCache(t) diff --git a/chainntnfs/btcdnotify/btcd_test.go b/chainntnfs/btcdnotify/btcd_test.go index 240c5a24ce..1cfbff731f 100644 --- a/chainntnfs/btcdnotify/btcd_test.go +++ b/chainntnfs/btcdnotify/btcd_test.go @@ -12,6 +12,7 @@ import ( "github.com/lightningnetwork/lnd/blockcache" "github.com/lightningnetwork/lnd/chainntnfs" "github.com/lightningnetwork/lnd/channeldb" + "github.com/lightningnetwork/lnd/lntest/unittest" "github.com/stretchr/testify/require" ) @@ -55,7 +56,7 @@ func setUpNotifier(t *testing.T, h *rpctest.Harness) *BtcdNotifier { rpcCfg := h.RPCConfig() notifier, err := New( - &rpcCfg, chainntnfs.NetParams, hintCache, hintCache, blockCache, + &rpcCfg, unittest.NetParams, hintCache, hintCache, blockCache, ) require.NoError(t, err, "unable to create notifier") if err := notifier.Start(); err != nil { @@ -73,8 +74,8 @@ func setUpNotifier(t *testing.T, h *rpctest.Harness) *BtcdNotifier { func TestHistoricalConfDetailsTxIndex(t *testing.T) { t.Parallel() - harness := chainntnfs.NewMiner( - t, chainntnfs.NetParams, []string{"--txindex"}, true, 25, + harness := unittest.NewMiner( + t, unittest.NetParams, []string{"--txindex"}, true, 25, ) notifier := setUpNotifier(t, harness) @@ -145,7 +146,7 @@ func TestHistoricalConfDetailsTxIndex(t *testing.T) { func TestHistoricalConfDetailsNoTxIndex(t *testing.T) { t.Parallel() - harness := chainntnfs.NewMiner(t, chainntnfs.NetParams, nil, true, 25) + harness := unittest.NewMiner(t, unittest.NetParams, nil, true, 25) notifier := setUpNotifier(t, harness) diff --git a/chainntnfs/test/test_interface.go b/chainntnfs/test/test_interface.go index a11f751c68..9a8a89879a 100644 --- a/chainntnfs/test/test_interface.go +++ b/chainntnfs/test/test_interface.go @@ -25,6 +25,7 @@ import ( "github.com/lightningnetwork/lnd/chainntnfs/btcdnotify" "github.com/lightningnetwork/lnd/chainntnfs/neutrinonotify" "github.com/lightningnetwork/lnd/channeldb" + "github.com/lightningnetwork/lnd/lntest/unittest" "github.com/lightningnetwork/lnd/lnutils" "github.com/stretchr/testify/require" ) @@ -1043,8 +1044,8 @@ func testReorgConf(miner *rpctest.Harness, notifier chainntnfs.TestChainNotifier, scriptDispatch bool, t *testing.T) { // Set up a new miner that we can use to cause a reorg. - miner2 := chainntnfs.NewMiner( - t, chainntnfs.NetParams, []string{"--txindex"}, false, 0, + miner2 := unittest.NewMiner( + t, unittest.NetParams, []string{"--txindex"}, false, 0, ) // We start by connecting the new miner to our original miner, @@ -1199,8 +1200,8 @@ func testReorgSpend(miner *rpctest.Harness, require.NoError(t, err, "unable to register for spend") // Set up a new miner that we can use to cause a reorg. - miner2 := chainntnfs.NewMiner( - t, chainntnfs.NetParams, []string{"--txindex"}, false, 0, + miner2 := unittest.NewMiner( + t, unittest.NetParams, []string{"--txindex"}, false, 0, ) // We start by connecting the new miner to our original miner, in order @@ -1517,8 +1518,8 @@ func testCatchUpOnMissedBlocksWithReorg(miner1 *rpctest.Harness, var wg sync.WaitGroup // Set up a new miner that we can use to cause a reorg. - miner2 := chainntnfs.NewMiner( - t, chainntnfs.NetParams, []string{"--txindex"}, false, 0, + miner2 := unittest.NewMiner( + t, unittest.NetParams, []string{"--txindex"}, false, 0, ) // We start by connecting the new miner to our original miner, @@ -1890,7 +1891,7 @@ func TestInterfaces(t *testing.T, targetBackEnd string) { // dedicated miner to generate blocks, cause re-orgs, etc. We'll set up // this node with a chain length of 125, so we have plenty of BTC to // play around with. - miner := chainntnfs.NewMiner(t, chainntnfs.NetParams, nil, true, 25) + miner := unittest.NewMiner(t, unittest.NetParams, nil, true, 25) rpcConfig := miner.RPCConfig() p2pAddr := miner.P2PAddress() @@ -1929,24 +1930,24 @@ func TestInterfaces(t *testing.T, targetBackEnd string) { switch notifierType { case "bitcoind": var bitcoindConn *chain.BitcoindConn - bitcoindConn = chainntnfs.NewBitcoindBackend( - t, chainntnfs.NetParams, p2pAddr, true, false, + bitcoindConn = unittest.NewBitcoindBackend( + t, unittest.NetParams, p2pAddr, true, false, ) newNotifier = func() (chainntnfs.TestChainNotifier, error) { return bitcoindnotify.New( - bitcoindConn, chainntnfs.NetParams, + bitcoindConn, unittest.NetParams, hintCache, hintCache, blockCache, ), nil } case "bitcoind-rpc-polling": var bitcoindConn *chain.BitcoindConn - bitcoindConn = chainntnfs.NewBitcoindBackend( - t, chainntnfs.NetParams, p2pAddr, true, true, + bitcoindConn = unittest.NewBitcoindBackend( + t, unittest.NetParams, p2pAddr, true, true, ) newNotifier = func() (chainntnfs.TestChainNotifier, error) { return bitcoindnotify.New( - bitcoindConn, chainntnfs.NetParams, + bitcoindConn, unittest.NetParams, hintCache, hintCache, blockCache, ), nil } @@ -1954,15 +1955,15 @@ func TestInterfaces(t *testing.T, targetBackEnd string) { case "btcd": newNotifier = func() (chainntnfs.TestChainNotifier, error) { return btcdnotify.New( - &rpcConfig, chainntnfs.NetParams, + &rpcConfig, unittest.NetParams, hintCache, hintCache, blockCache, ) } case "neutrino": var spvNode *neutrino.ChainService - spvNode = chainntnfs.NewNeutrinoBackend( - t, chainntnfs.NetParams, p2pAddr, + spvNode = unittest.NewNeutrinoBackend( + t, unittest.NetParams, p2pAddr, ) newNotifier = func() (chainntnfs.TestChainNotifier, error) { return neutrinonotify.New( diff --git a/chainntnfs/test_utils.go b/chainntnfs/test_utils.go index a0be772cfe..b14218dd7a 100644 --- a/chainntnfs/test_utils.go +++ b/chainntnfs/test_utils.go @@ -6,25 +6,17 @@ package chainntnfs import ( "errors" "fmt" - "os/exec" - "path/filepath" "testing" "time" "github.com/btcsuite/btcd/btcec/v2" "github.com/btcsuite/btcd/btcjson" "github.com/btcsuite/btcd/btcutil" - "github.com/btcsuite/btcd/chaincfg" "github.com/btcsuite/btcd/chaincfg/chainhash" "github.com/btcsuite/btcd/integration/rpctest" "github.com/btcsuite/btcd/txscript" "github.com/btcsuite/btcd/wire" - "github.com/btcsuite/btcwallet/chain" - "github.com/btcsuite/btcwallet/walletdb" - "github.com/lightninglabs/neutrino" - "github.com/lightningnetwork/lnd/kvdb" - "github.com/lightningnetwork/lnd/lntest/port" - "github.com/lightningnetwork/lnd/lntest/wait" + "github.com/lightningnetwork/lnd/lntest/unittest" "github.com/stretchr/testify/require" ) @@ -35,10 +27,6 @@ var ( TrickleInterval = 10 * time.Millisecond ) -var ( - NetParams = &chaincfg.RegressionNetParams -) - // randPubKeyHashScript generates a P2PKH script that pays to the public key of // a randomly-generated private key. func randPubKeyHashScript() ([]byte, *btcec.PrivateKey, error) { @@ -48,7 +36,9 @@ func randPubKeyHashScript() ([]byte, *btcec.PrivateKey, error) { } pubKeyHash := btcutil.Hash160(privKey.PubKey().SerializeCompressed()) - addrScript, err := btcutil.NewAddressPubKeyHash(pubKeyHash, NetParams) + addrScript, err := btcutil.NewAddressPubKeyHash( + pubKeyHash, unittest.NetParams, + ) if err != nil { return nil, nil, err } @@ -162,169 +152,3 @@ func CreateSpendTx(t *testing.T, prevOutPoint *wire.OutPoint, return spendingTx } - -// NewMiner spawns testing harness backed by a btcd node that can serve as a -// miner. -func NewMiner(t *testing.T, netParams *chaincfg.Params, extraArgs []string, - createChain bool, spendableOutputs uint32) *rpctest.Harness { - - t.Helper() - - // Add the trickle interval argument to the extra args. - trickle := fmt.Sprintf("--trickleinterval=%v", TrickleInterval) - extraArgs = append(extraArgs, trickle) - - node, err := rpctest.New(netParams, nil, extraArgs, "") - require.NoError(t, err, "unable to create backend node") - t.Cleanup(func() { - require.NoError(t, node.TearDown()) - }) - - // We want to overwrite some of the connection settings to make the - // tests more robust. We might need to restart the backend while there - // are already blocks present, which will take a bit longer than the - // 1 second the default settings amount to. Doubling both values will - // give us retries up to 4 seconds. - node.MaxConnRetries = rpctest.DefaultMaxConnectionRetries * 2 - node.ConnectionRetryTimeout = rpctest.DefaultConnectionRetryTimeout * 2 - - if err := node.SetUp(createChain, spendableOutputs); err != nil { - t.Fatalf("unable to set up backend node: %v", err) - } - - return node -} - -// NewBitcoindBackend spawns a new bitcoind node that connects to a miner at the -// specified address. The txindex boolean can be set to determine whether the -// backend node should maintain a transaction index. The rpcpolling boolean -// can be set to determine whether bitcoind's RPC polling interface should be -// used for block and tx notifications or if its ZMQ interface should be used. -// A connection to the newly spawned bitcoind node is returned. -func NewBitcoindBackend(t *testing.T, netParams *chaincfg.Params, - minerAddr string, txindex, rpcpolling bool) *chain.BitcoindConn { - - t.Helper() - - tempBitcoindDir := t.TempDir() - - rpcPort := port.NextAvailablePort() - zmqBlockPort := port.NextAvailablePort() - zmqTxPort := port.NextAvailablePort() - zmqBlockHost := fmt.Sprintf("tcp://127.0.0.1:%d", zmqBlockPort) - zmqTxHost := fmt.Sprintf("tcp://127.0.0.1:%d", zmqTxPort) - - args := []string{ - "-connect=" + minerAddr, - "-datadir=" + tempBitcoindDir, - "-regtest", - "-rpcauth=weks:469e9bb14ab2360f8e226efed5ca6fd$507c670e800a952" + - "84294edb5773b05544b220110063096c221be9933c82d38e1", - fmt.Sprintf("-rpcport=%d", rpcPort), - "-disablewallet", - "-zmqpubrawblock=" + zmqBlockHost, - "-zmqpubrawtx=" + zmqTxHost, - } - if txindex { - args = append(args, "-txindex") - } - - bitcoind := exec.Command("bitcoind", args...) - if err := bitcoind.Start(); err != nil { - t.Fatalf("unable to start bitcoind: %v", err) - } - t.Cleanup(func() { - _ = bitcoind.Process.Kill() - _ = bitcoind.Wait() - }) - - // Wait for the bitcoind instance to start up. - host := fmt.Sprintf("127.0.0.1:%d", rpcPort) - cfg := &chain.BitcoindConfig{ - ChainParams: netParams, - Host: host, - User: "weks", - Pass: "weks", - // Fields only required for pruned nodes, not needed for these - // tests. - Dialer: nil, - PrunedModeMaxPeers: 0, - } - - 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, - } - } - - var conn *chain.BitcoindConn - err := wait.NoError(func() error { - var err error - conn, err = chain.NewBitcoindConn(cfg) - if err != nil { - return err - } - - return conn.Start() - }, 10*time.Second) - if err != nil { - t.Fatalf("unable to establish connection to bitcoind at %v: "+ - "%v", tempBitcoindDir, err) - } - t.Cleanup(conn.Stop) - - return conn -} - -// NewNeutrinoBackend spawns a new neutrino node that connects to a miner at -// the specified address. -func NewNeutrinoBackend(t *testing.T, netParams *chaincfg.Params, - minerAddr string) *neutrino.ChainService { - - t.Helper() - - spvDir := t.TempDir() - - dbName := filepath.Join(spvDir, "neutrino.db") - spvDatabase, err := walletdb.Create( - "bdb", dbName, true, kvdb.DefaultDBTimeout, - ) - if err != nil { - t.Fatalf("unable to create walletdb: %v", err) - } - t.Cleanup(func() { - spvDatabase.Close() - }) - - // Create an instance of neutrino connected to the running btcd - // instance. - spvConfig := neutrino.Config{ - DataDir: spvDir, - Database: spvDatabase, - ChainParams: *netParams, - ConnectPeers: []string{minerAddr}, - } - spvNode, err := neutrino.NewChainService(spvConfig) - if err != nil { - t.Fatalf("unable to create neutrino: %v", err) - } - - // We'll also wait for the instance to sync up fully to the chain - // generated by the btcd instance. - spvNode.Start() - for !spvNode.IsCurrent() { - time.Sleep(time.Millisecond * 100) - } - t.Cleanup(func() { - spvNode.Stop() - }) - - return spvNode -} diff --git a/lntest/unittest/backend.go b/lntest/unittest/backend.go new file mode 100644 index 0000000000..2f9c123636 --- /dev/null +++ b/lntest/unittest/backend.go @@ -0,0 +1,207 @@ +package unittest + +import ( + "fmt" + "os/exec" + "path/filepath" + "testing" + "time" + + "github.com/btcsuite/btcd/chaincfg" + "github.com/btcsuite/btcd/integration/rpctest" + "github.com/btcsuite/btcwallet/chain" + "github.com/btcsuite/btcwallet/walletdb" + "github.com/lightninglabs/neutrino" + "github.com/lightningnetwork/lnd/kvdb" + "github.com/lightningnetwork/lnd/lntest/port" + "github.com/lightningnetwork/lnd/lntest/wait" + "github.com/stretchr/testify/require" +) + +var ( + // TrickleInterval is the interval at which the miner should trickle + // transactions to its peers. We'll set it small to ensure the miner + // propagates transactions quickly in the tests. + TrickleInterval = 10 * time.Millisecond +) + +var ( + // NetParams are the default network parameters for the tests. + NetParams = &chaincfg.RegressionNetParams +) + +// NewMiner spawns testing harness backed by a btcd node that can serve as a +// miner. +func NewMiner(t *testing.T, netParams *chaincfg.Params, extraArgs []string, + createChain bool, spendableOutputs uint32) *rpctest.Harness { + + t.Helper() + + // Add the trickle interval argument to the extra args. + trickle := fmt.Sprintf("--trickleinterval=%v", TrickleInterval) + extraArgs = append(extraArgs, trickle) + + node, err := rpctest.New(netParams, nil, extraArgs, "") + require.NoError(t, err, "unable to create backend node") + t.Cleanup(func() { + require.NoError(t, node.TearDown()) + }) + + // We want to overwrite some of the connection settings to make the + // tests more robust. We might need to restart the backend while there + // are already blocks present, which will take a bit longer than the + // 1 second the default settings amount to. Doubling both values will + // give us retries up to 4 seconds. + node.MaxConnRetries = rpctest.DefaultMaxConnectionRetries * 2 + node.ConnectionRetryTimeout = rpctest.DefaultConnectionRetryTimeout * 2 + + if err := node.SetUp(createChain, spendableOutputs); err != nil { + t.Fatalf("unable to set up backend node: %v", err) + } + + return node +} + +// NewBitcoindBackend spawns a new bitcoind node that connects to a miner at the +// specified address. The txindex boolean can be set to determine whether the +// backend node should maintain a transaction index. The rpcpolling boolean +// can be set to determine whether bitcoind's RPC polling interface should be +// used for block and tx notifications or if its ZMQ interface should be used. +// A connection to the newly spawned bitcoind node is returned. +func NewBitcoindBackend(t *testing.T, netParams *chaincfg.Params, + minerAddr string, txindex, rpcpolling bool) *chain.BitcoindConn { + + t.Helper() + + tempBitcoindDir := t.TempDir() + + rpcPort := port.NextAvailablePort() + zmqBlockPort := port.NextAvailablePort() + zmqTxPort := port.NextAvailablePort() + zmqBlockHost := fmt.Sprintf("tcp://127.0.0.1:%d", zmqBlockPort) + zmqTxHost := fmt.Sprintf("tcp://127.0.0.1:%d", zmqTxPort) + + args := []string{ + "-connect=" + minerAddr, + "-datadir=" + tempBitcoindDir, + "-regtest", + "-rpcauth=weks:469e9bb14ab2360f8e226efed5ca6fd$507c670e800a95" + + "284294edb5773b05544b220110063096c221be9933c82d38e1", + fmt.Sprintf("-rpcport=%d", rpcPort), + "-disablewallet", + "-zmqpubrawblock=" + zmqBlockHost, + "-zmqpubrawtx=" + zmqTxHost, + } + if txindex { + args = append(args, "-txindex") + } + + bitcoind := exec.Command("bitcoind", args...) + if err := bitcoind.Start(); err != nil { + t.Fatalf("unable to start bitcoind: %v", err) + } + t.Cleanup(func() { + _ = bitcoind.Process.Kill() + _ = bitcoind.Wait() + }) + + // Wait for the bitcoind instance to start up. + time.Sleep(time.Second) + + host := fmt.Sprintf("127.0.0.1:%d", rpcPort) + cfg := &chain.BitcoindConfig{ + ChainParams: netParams, + Host: host, + User: "weks", + Pass: "weks", + // Fields only required for pruned nodes, not needed for these + // tests. + Dialer: nil, + PrunedModeMaxPeers: 0, + } + + 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, + } + } + + var conn *chain.BitcoindConn + err := wait.NoError(func() error { + var err error + conn, err = chain.NewBitcoindConn(cfg) + if err != nil { + return err + } + + return conn.Start() + }, 10*time.Second) + if err != nil { + t.Fatalf("unable to establish connection to bitcoind at %v: "+ + "%v", tempBitcoindDir, err) + } + t.Cleanup(conn.Stop) + + return conn +} + +// NewNeutrinoBackend spawns a new neutrino node that connects to a miner at +// the specified address. +func NewNeutrinoBackend(t *testing.T, netParams *chaincfg.Params, + minerAddr string) *neutrino.ChainService { + + t.Helper() + + spvDir := t.TempDir() + + dbName := filepath.Join(spvDir, "neutrino.db") + spvDatabase, err := walletdb.Create( + "bdb", dbName, true, kvdb.DefaultDBTimeout, + ) + if err != nil { + t.Fatalf("unable to create walletdb: %v", err) + } + t.Cleanup(func() { + spvDatabase.Close() + }) + + // Create an instance of neutrino connected to the running btcd + // instance. + spvConfig := neutrino.Config{ + DataDir: spvDir, + Database: spvDatabase, + ChainParams: *netParams, + ConnectPeers: []string{minerAddr}, + } + spvNode, err := neutrino.NewChainService(spvConfig) + if err != nil { + t.Fatalf("unable to create neutrino: %v", err) + } + + // We'll also wait for the instance to sync up fully to the chain + // generated by the btcd instance. + _ = spvNode.Start() + for !spvNode.IsCurrent() { + time.Sleep(time.Millisecond * 100) + } + t.Cleanup(func() { + _ = spvNode.Stop() + }) + + return spvNode +} + +func init() { + // Before we start any node, we need to make sure that any btcd or + // bitcoind node that is started through the RPC harness uses a unique + // port as well to avoid any port collisions. + rpctest.ListenAddressGenerator = + port.GenerateSystemUniqueListenerAddresses +} diff --git a/lnwallet/btcwallet/signer_test.go b/lnwallet/btcwallet/signer_test.go index 7e5a3a20ee..a0e4bba6de 100644 --- a/lnwallet/btcwallet/signer_test.go +++ b/lnwallet/btcwallet/signer_test.go @@ -18,8 +18,8 @@ import ( "github.com/btcsuite/btcwallet/chain" "github.com/btcsuite/btcwallet/waddrmgr" "github.com/lightningnetwork/lnd/blockcache" - "github.com/lightningnetwork/lnd/chainntnfs" "github.com/lightningnetwork/lnd/input" + "github.com/lightningnetwork/lnd/lntest/unittest" "github.com/lightningnetwork/lnd/lnwallet" "github.com/stretchr/testify/require" ) @@ -326,7 +326,7 @@ func newTestWallet(t *testing.T, netParams *chaincfg.Params, func getChainBackend(t *testing.T, netParams *chaincfg.Params) (chain.Interface, *rpctest.Harness) { - miningNode := chainntnfs.NewMiner( + miningNode := unittest.NewMiner( t, netParams, []string{"--txindex"}, true, 25, ) diff --git a/lnwallet/test/test_interface.go b/lnwallet/test/test_interface.go index 42f63be51f..c769172c10 100644 --- a/lnwallet/test/test_interface.go +++ b/lnwallet/test/test_interface.go @@ -40,6 +40,7 @@ import ( "github.com/lightningnetwork/lnd/keychain" "github.com/lightningnetwork/lnd/kvdb" "github.com/lightningnetwork/lnd/labels" + "github.com/lightningnetwork/lnd/lntest/unittest" "github.com/lightningnetwork/lnd/lntest/wait" "github.com/lightningnetwork/lnd/lnwallet" "github.com/lightningnetwork/lnd/lnwallet/btcwallet" @@ -2120,7 +2121,7 @@ func testReorgWalletBalance(r *rpctest.Harness, w *lnwallet.LightningWallet, // Now we cause a reorganization as follows. // Step 1: create a new miner and start it. - r2 := chainntnfs.NewMiner( + r2 := unittest.NewMiner( t, r.ActiveNet, []string{"--txindex"}, false, 0, ) newBalance, err := w.ConfirmedBalance(1, lnwallet.DefaultAccountName) @@ -3101,7 +3102,7 @@ func TestLightningWallet(t *testing.T, targetBackEnd string) { // dedicated miner to generate blocks, cause re-orgs, etc. We'll set // up this node with a chain length of 125, so we have plenty of BTC // to play around with. - miningNode := chainntnfs.NewMiner( + miningNode := unittest.NewMiner( t, netParams, []string{"--txindex"}, true, 25, ) diff --git a/routing/chainview/interface_test.go b/routing/chainview/interface_test.go index c3bf9ac063..d8d94b93d8 100644 --- a/routing/chainview/interface_test.go +++ b/routing/chainview/interface_test.go @@ -25,9 +25,9 @@ import ( _ "github.com/btcsuite/btcwallet/walletdb/bdb" // Required to register the boltdb walletdb implementation. "github.com/lightninglabs/neutrino" "github.com/lightningnetwork/lnd/blockcache" - "github.com/lightningnetwork/lnd/chainntnfs" "github.com/lightningnetwork/lnd/channeldb" "github.com/lightningnetwork/lnd/kvdb" + "github.com/lightningnetwork/lnd/lntest/unittest" "github.com/lightningnetwork/lnd/lntest/wait" "github.com/stretchr/testify/require" ) @@ -482,7 +482,7 @@ func testFilterBlockDisconnected(node *rpctest.Harness, // Create a node that has a shorter chain than the main chain, so we // can trigger a reorg. - reorgNode := chainntnfs.NewMiner( + reorgNode := unittest.NewMiner( t, netParams, []string{"--txindex"}, true, 5, ) @@ -984,7 +984,7 @@ func TestFilteredChainView(t *testing.T) { // dedicated miner to generate blocks, cause re-orgs, etc. We'll set up // this node with a chain length of 125, so we have plenty of BTC to // play around with. - miner := chainntnfs.NewMiner( + miner := unittest.NewMiner( t, netParams, []string{"--txindex"}, true, 25, ) From aa811c784a9480f9546b9b267f928afe08d00c58 Mon Sep 17 00:00:00 2001 From: Oliver Gugger Date: Fri, 15 Mar 2024 12:54:37 +0100 Subject: [PATCH 08/13] lnwallet+routing: use chainntnfs.NewBitcoindBackend Since we fixed a number of issues in chainntnfs.NewBitcoindBackend that makes it compatible with bitcoind v26.0, we now want to use that function in all our unit tests. --- lnwallet/test/test_interface.go | 183 ++-------------------- routing/chainview/interface_test.go | 225 +++------------------------- 2 files changed, 35 insertions(+), 373 deletions(-) diff --git a/lnwallet/test/test_interface.go b/lnwallet/test/test_interface.go index c769172c10..17cfa53f51 100644 --- a/lnwallet/test/test_interface.go +++ b/lnwallet/test/test_interface.go @@ -6,12 +6,10 @@ import ( "encoding/hex" "fmt" "net" - "os/exec" "path/filepath" "reflect" "runtime" "strings" - "sync/atomic" "testing" "time" @@ -41,7 +39,6 @@ import ( "github.com/lightningnetwork/lnd/kvdb" "github.com/lightningnetwork/lnd/labels" "github.com/lightningnetwork/lnd/lntest/unittest" - "github.com/lightningnetwork/lnd/lntest/wait" "github.com/lightningnetwork/lnd/lnwallet" "github.com/lightningnetwork/lnd/lnwallet/btcwallet" "github.com/lightningnetwork/lnd/lnwallet/chainfee" @@ -97,35 +94,6 @@ var ( defaultMaxLocalCsvDelay uint16 = 10000 ) -var ( - // lastPort is the last port determined to be free for use by a new - // bitcoind server. It should be used atomically. - lastPort uint32 = 1024 -) - -// getFreePort returns the first port that is available for listening by a new -// embedded etcd server. It panics if no port is found and the maximum available -// TCP port is reached. -func getFreePort() int { - port := atomic.AddUint32(&lastPort, 1) - for port < 65535 { - // If there are no errors while attempting to listen on this - // port, close the socket and return it as available. - addr := fmt.Sprintf("127.0.0.1:%d", port) - l, err := net.Listen("tcp4", addr) - if err == nil { - err := l.Close() - if err == nil { - return int(port) - } - } - port = atomic.AddUint32(&lastPort, 1) - } - - // No ports available? Must be a mistake. - panic("no ports available for listening") -} - // assertProperBalance asserts than the total value of the unspent outputs // within the wallet are *exactly* amount. If unable to retrieve the current // balance, or the assertion fails, the test will halt with a fatal error. @@ -3182,15 +3150,19 @@ func runTests(t *testing.T, walletDriver *lnwallet.WalletDriver, var aliceClient, bobClient chain.Interface switch backEnd { case "btcd": - aliceClient, err = chain.NewRPCClient(netParams, - rpcConfig.Host, rpcConfig.User, rpcConfig.Pass, - rpcConfig.Certificates, false, 20) + aliceClient, err = chain.NewRPCClient( + netParams, rpcConfig.Host, rpcConfig.User, + rpcConfig.Pass, rpcConfig.Certificates, false, + 20, + ) if err != nil { t.Fatalf("unable to make chain rpc: %v", err) } - bobClient, err = chain.NewRPCClient(netParams, - rpcConfig.Host, rpcConfig.User, rpcConfig.Pass, - rpcConfig.Certificates, false, 20) + bobClient, err = chain.NewRPCClient( + netParams, rpcConfig.Host, rpcConfig.User, + rpcConfig.Pass, rpcConfig.Certificates, false, + 20, + ) if err != nil { t.Fatalf("unable to make chain rpc: %v", err) } @@ -3262,78 +3234,10 @@ func runTests(t *testing.T, walletDriver *lnwallet.WalletDriver, case "bitcoind": // Start a bitcoind instance. - tempBitcoindDir := t.TempDir() - - rpcPort := getFreePort() - zmqBlockPort := getFreePort() - zmqTxPort := getFreePort() - zmqBlockHost := fmt.Sprintf("tcp://127.0.0.1:%d", - zmqBlockPort) - zmqTxHost := fmt.Sprintf("tcp://127.0.0.1:%d", - zmqTxPort) - - bitcoind := exec.Command( - "bitcoind", - "-datadir="+tempBitcoindDir, - "-regtest", - "-connect="+miningNode.P2PAddress(), - "-txindex", - "-rpcauth=weks:469e9bb14ab2360f8e226efed5ca6f"+ - "d$507c670e800a95284294edb5773b05544b"+ - "220110063096c221be9933c82d38e1", - fmt.Sprintf("-rpcport=%d", rpcPort), - "-disablewallet", - "-zmqpubrawblock="+zmqBlockHost, - "-zmqpubrawtx="+zmqTxHost, + chainConn := unittest.NewBitcoindBackend( + t, unittest.NetParams, miningNode.P2PAddress(), + true, false, ) - err = bitcoind.Start() - if err != nil { - t.Fatalf("couldn't start bitcoind: %v", err) - } - - // Sanity check to ensure that the process did in fact - // start. - if bitcoind.Process == nil { - t.Fatalf("bitcoind cmd Process is not set " + - "after Start") - } - - defer func() { - _ = bitcoind.Process.Kill() - _ = bitcoind.Wait() - }() - - // Wait for the bitcoind instance to start up. - - host := fmt.Sprintf("127.0.0.1:%d", rpcPort) - var chainConn *chain.BitcoindConn - err = wait.NoError(func() error { - chainConn, err = chain.NewBitcoindConn(&chain.BitcoindConfig{ - ChainParams: netParams, - Host: host, - User: "weks", - Pass: "weks", - ZMQConfig: &chain.ZMQConfig{ - ZMQBlockHost: zmqBlockHost, - ZMQTxHost: zmqTxHost, - ZMQReadDeadline: 5 * time.Second, - }, - // Fields only required for pruned nodes, not - // needed for these tests. - Dialer: nil, - PrunedModeMaxPeers: 0, - }) - if err != nil { - return err - } - - return chainConn.Start() - }, 10*time.Second) - if err != nil { - t.Fatalf("unable to establish connection to "+ - "bitcoind: %v", err) - } - defer chainConn.Stop() // Create a btcwallet bitcoind client for both Alice and // Bob. @@ -3342,65 +3246,10 @@ func runTests(t *testing.T, walletDriver *lnwallet.WalletDriver, case "bitcoind-rpc-polling": // Start a bitcoind instance. - tempBitcoindDir := t.TempDir() - rpcPort := getFreePort() - bitcoind := exec.Command( - "bitcoind", - "-datadir="+tempBitcoindDir, - "-regtest", - "-connect="+miningNode.P2PAddress(), - "-txindex", - "-rpcauth=weks:469e9bb14ab2360f8e226efed5ca6f"+ - "d$507c670e800a95284294edb5773b05544b"+ - "220110063096c221be9933c82d38e1", - fmt.Sprintf("-rpcport=%d", rpcPort), - "-disablewallet", + chainConn := unittest.NewBitcoindBackend( + t, unittest.NetParams, miningNode.P2PAddress(), + true, true, ) - err = bitcoind.Start() - if err != nil { - t.Fatalf("couldn't start bitcoind: %v", err) - } - defer func() { - _ = bitcoind.Process.Kill() - _ = bitcoind.Wait() - }() - - // Sanity check to ensure that the process did in fact - // start. - if bitcoind.Process == nil { - t.Fatalf("bitcoind cmd Process is not set " + - "after Start") - } - - // Wait for the bitcoind instance to start up. - host := fmt.Sprintf("127.0.0.1:%d", rpcPort) - var chainConn *chain.BitcoindConn - err = wait.NoError(func() error { - chainConn, err = chain.NewBitcoindConn(&chain.BitcoindConfig{ - ChainParams: netParams, - Host: host, - User: "weks", - Pass: "weks", - PollingConfig: &chain.PollingConfig{ - BlockPollingInterval: time.Millisecond * 20, - TxPollingInterval: time.Millisecond * 20, - }, - // Fields only required for pruned nodes, not - // needed for these tests. - Dialer: nil, - PrunedModeMaxPeers: 0, - }) - if err != nil { - return err - } - - return chainConn.Start() - }, 10*time.Second) - if err != nil { - t.Fatalf("unable to establish connection to "+ - "bitcoind: %v", err) - } - defer chainConn.Stop() // Create a btcwallet bitcoind client for both Alice and // Bob. diff --git a/routing/chainview/interface_test.go b/routing/chainview/interface_test.go index d8d94b93d8..3d97ff2c35 100644 --- a/routing/chainview/interface_test.go +++ b/routing/chainview/interface_test.go @@ -3,11 +3,8 @@ package chainview import ( "bytes" "fmt" - "net" - "os/exec" "path/filepath" "runtime" - "sync/atomic" "testing" "time" @@ -20,7 +17,6 @@ import ( "github.com/btcsuite/btcd/rpcclient" "github.com/btcsuite/btcd/txscript" "github.com/btcsuite/btcd/wire" - "github.com/btcsuite/btcwallet/chain" "github.com/btcsuite/btcwallet/walletdb" _ "github.com/btcsuite/btcwallet/walletdb/bdb" // Required to register the boltdb walletdb implementation. "github.com/lightninglabs/neutrino" @@ -50,35 +46,6 @@ var ( testScript, _ = txscript.PayToAddrScript(testAddr) ) -var ( - // lastPort is the last port determined to be free for use by a new - // bitcoind server. It should be used atomically. - lastPort uint32 = 1024 -) - -// getFreePort returns the first port that is available for listening by a new -// embedded etcd server. It panics if no port is found and the maximum available -// TCP port is reached. -func getFreePort() int { - port := atomic.AddUint32(&lastPort, 1) - for port < 65535 { - // If there are no errors while attempting to listen on this - // port, close the socket and return it as available. - addr := fmt.Sprintf("127.0.0.1:%d", port) - l, err := net.Listen("tcp4", addr) - if err == nil { - err := l.Close() - if err == nil { - return int(port) - } - } - port = atomic.AddUint32(&lastPort, 1) - } - - // No ports available? Must be a mistake. - panic("no ports available for listening") -} - func waitForMempoolTx(r *rpctest.Harness, txid *chainhash.Hash) error { var found bool var tx *btcutil.Tx @@ -221,8 +188,10 @@ func testFilterBlockNotifications(node *rpctest.Harness, // filtered transaction as the filter hasn't been update. select { case filteredBlock := <-blockChan: - assertFilteredBlock(t, filteredBlock, currentHeight, - newBlockHashes[0], []*chainhash.Hash{}) + assertFilteredBlock( + t, filteredBlock, currentHeight, + newBlockHashes[0], []*chainhash.Hash{}, + ) case <-time.After(time.Second * 20): t.Fatalf("filtered block notification didn't arrive") } @@ -697,95 +666,14 @@ var interfaceImpls = []struct { { name: "bitcoind_zmq", chainViewInit: func(t *testing.T, _ rpcclient.ConnConfig, - p2pAddr string, bestHeight int32) (FilteredChainView, error) { + p2pAddr string, bestHeight int32) (FilteredChainView, + error) { // Start a bitcoind instance. - tempBitcoindDir := t.TempDir() - zmqBlockHost := "ipc:///" + tempBitcoindDir + "/blocks.socket" - zmqTxHost := "ipc:///" + tempBitcoindDir + "/tx.socket" - - rpcPort := getFreePort() - bitcoind := exec.Command( - "bitcoind", - "-datadir="+tempBitcoindDir, - "-regtest", - "-connect="+p2pAddr, - "-txindex", - "-rpcauth=weks:469e9bb14ab2360f8e226efed5ca6f"+ - "d$507c670e800a95284294edb5773b05544b"+ - "220110063096c221be9933c82d38e1", - fmt.Sprintf("-rpcport=%d", rpcPort), - "-disablewallet", - "-zmqpubrawblock="+zmqBlockHost, - "-zmqpubrawtx="+zmqTxHost, + chainConn := unittest.NewBitcoindBackend( + t, unittest.NetParams, p2pAddr, true, + false, ) - err := bitcoind.Start() - if err != nil { - return nil, err - } - - // Sanity check to ensure that the process did in fact - // start. - if bitcoind.Process == nil { - return nil, fmt.Errorf("bitcoind cmd " + - "Process is not set after Start") - } - - t.Cleanup(func() { - _ = bitcoind.Process.Kill() - _ = bitcoind.Wait() - }) - - host := fmt.Sprintf("127.0.0.1:%d", rpcPort) - cfg := &chain.BitcoindConfig{ - ChainParams: &chaincfg.RegressionNetParams, - Host: host, - User: "weks", - Pass: "weks", - ZMQConfig: &chain.ZMQConfig{ - ZMQBlockHost: zmqBlockHost, - ZMQTxHost: zmqTxHost, - ZMQReadDeadline: 5 * time.Second, - }, - // Fields only required for pruned nodes, not - // needed for these tests. - Dialer: nil, - PrunedModeMaxPeers: 0, - } - - var chainConn *chain.BitcoindConn - err = wait.NoError(func() error { - chainConn, err = chain.NewBitcoindConn(cfg) - if err != nil { - return err - } - - err = chainConn.Start() - if err != nil { - return err - } - - client := chainConn.NewBitcoindClient() - _, currentHeight, err := client.GetBestBlock() - if err != nil { - return err - } - - if currentHeight < bestHeight { - return fmt.Errorf("not synced yet") - } - - return nil - }, 10*time.Second) - if err != nil { - return nil, fmt.Errorf("unable to "+ - "establish connection to bitcoind: %v", - err) - } - t.Cleanup(func() { - chainConn.Stop() - }) - blockCache := blockcache.NewBlockCache(10000) chainView := NewBitcoindFilteredChainView( @@ -798,91 +686,14 @@ var interfaceImpls = []struct { { name: "bitcoind_polling", chainViewInit: func(t *testing.T, _ rpcclient.ConnConfig, - p2pAddr string, bestHeight int32) (FilteredChainView, error) { - - // Start a bitcoind instance. - tempBitcoindDir := t.TempDir() - - rpcPort := getFreePort() - bitcoind := exec.Command( - "bitcoind", - "-datadir="+tempBitcoindDir, - "-regtest", - "-connect="+p2pAddr, - "-txindex", - "-rpcauth=weks:469e9bb14ab2360f8e226efed5ca6f"+ - "d$507c670e800a95284294edb5773b05544b"+ - "220110063096c221be9933c82d38e1", - fmt.Sprintf("-rpcport=%d", rpcPort), - "-disablewallet", - ) - err := bitcoind.Start() - if err != nil { - return nil, err - } - - // Sanity check to ensure that the process did in fact - // start. - if bitcoind.Process == nil { - return nil, fmt.Errorf("bitcoind cmd " + - "Process is not set after Start") - } - - t.Cleanup(func() { - _ = bitcoind.Process.Kill() - _ = bitcoind.Wait() - }) - - host := fmt.Sprintf("127.0.0.1:%d", rpcPort) - cfg := &chain.BitcoindConfig{ - ChainParams: &chaincfg.RegressionNetParams, - Host: host, - User: "weks", - Pass: "weks", - PollingConfig: &chain.PollingConfig{ - BlockPollingInterval: time.Millisecond * 100, - TxPollingInterval: time.Millisecond * 100, - }, - // Fields only required for pruned nodes, not - // needed for these tests. - Dialer: nil, - PrunedModeMaxPeers: 0, - } + p2pAddr string, bestHeight int32) (FilteredChainView, + error) { // Wait for the bitcoind instance to start up. - var chainConn *chain.BitcoindConn - err = wait.NoError(func() error { - chainConn, err = chain.NewBitcoindConn(cfg) - if err != nil { - return err - } - - err = chainConn.Start() - if err != nil { - return err - } - - client := chainConn.NewBitcoindClient() - _, currentHeight, err := client.GetBestBlock() - if err != nil { - return err - } - - if currentHeight < bestHeight { - return fmt.Errorf("not synced yet") - } - - return nil - }, 10*time.Second) - if err != nil { - return nil, fmt.Errorf("unable to "+ - "establish connection to bitcoind: %v", - err) - } - t.Cleanup(func() { - chainConn.Stop() - }) - + chainConn := unittest.NewBitcoindBackend( + t, unittest.NetParams, p2pAddr, true, + true, + ) blockCache := blockcache.NewBlockCache(10000) chainView := NewBitcoindFilteredChainView( @@ -895,7 +706,8 @@ var interfaceImpls = []struct { { name: "p2p_neutrino", chainViewInit: func(t *testing.T, _ rpcclient.ConnConfig, - p2pAddr string, bestHeight int32) (FilteredChainView, error) { + p2pAddr string, bestHeight int32) (FilteredChainView, + error) { spvDir := t.TempDir() @@ -964,7 +776,8 @@ var interfaceImpls = []struct { { name: "btcd_websockets", chainViewInit: func(_ *testing.T, config rpcclient.ConnConfig, - p2pAddr string, bestHeight int32) (FilteredChainView, error) { + p2pAddr string, bestHeight int32) (FilteredChainView, + error) { blockCache := blockcache.NewBlockCache(10000) chainView, err := NewBtcdFilteredChainView( From b1b32d902697cb8bd5434afb4869f15a738e4939 Mon Sep 17 00:00:00 2001 From: Oliver Gugger Date: Fri, 15 Mar 2024 12:54:38 +0100 Subject: [PATCH 09/13] make: add nocache and verbose arguments To make it easy to add "-test.v" and "-test.count=1" to the unit tests, we add two new make flags. --- make/testing_flags.mk | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/make/testing_flags.mk b/make/testing_flags.mk index df20cdb7ee..3fecb073a3 100644 --- a/make/testing_flags.mk +++ b/make/testing_flags.mk @@ -93,6 +93,14 @@ else TEST_FLAGS += -test.timeout=180m endif +ifneq ($(verbose),) +TEST_FLAGS += -test.v +endif + +ifneq ($(nocache),) +TEST_FLAGS += -test.count=1 +endif + GOLIST := go list -tags="$(DEV_TAGS)" -deps $(PKG)/... | grep '$(PKG)'| grep -v '/vendor/' GOLISTCOVER := $(shell go list -tags="$(DEV_TAGS)" -deps -f '{{.ImportPath}}' ./... | grep '$(PKG)' | sed -e 's/^$(ESCPKG)/./') From 77a82309f5528b804d42019d728908376e76b514 Mon Sep 17 00:00:00 2001 From: Oliver Gugger Date: Fri, 15 Mar 2024 12:54:39 +0100 Subject: [PATCH 10/13] docs: add release notes --- docs/release-notes/release-notes-0.18.0.md | 3 +++ 1 file changed, 3 insertions(+) diff --git a/docs/release-notes/release-notes-0.18.0.md b/docs/release-notes/release-notes-0.18.0.md index d7a4728139..46ef1e3cf2 100644 --- a/docs/release-notes/release-notes-0.18.0.md +++ b/docs/release-notes/release-notes-0.18.0.md @@ -340,6 +340,9 @@ * Added fuzz tests for [onion errors](https://github.com/lightningnetwork/lnd/pull/7669). +* Fixed stability and [compatibility of unit tests with `bitcoind + v26.0`](https://github.com/lightningnetwork/lnd/pull/8273). + ## Database * [Add context to InvoiceDB From ab422ba1845bed2ca3af64609412ea03b12b7195 Mon Sep 17 00:00:00 2001 From: Oliver Gugger Date: Fri, 15 Mar 2024 12:54:41 +0100 Subject: [PATCH 11/13] lntest: give chain backend more time to get ready This is a commit to attempt to fix Travis which runs on ARM64. --- lntest/harness_setup.go | 12 ++++++++++-- 1 file changed, 10 insertions(+), 2 deletions(-) diff --git a/lntest/harness_setup.go b/lntest/harness_setup.go index 61eac5d2fa..8aed3b880d 100644 --- a/lntest/harness_setup.go +++ b/lntest/harness_setup.go @@ -7,6 +7,7 @@ import ( "github.com/btcsuite/btcd/integration/rpctest" "github.com/lightningnetwork/lnd/lntest/node" + "github.com/lightningnetwork/lnd/lntest/wait" "github.com/stretchr/testify/require" ) @@ -41,8 +42,15 @@ func SetupHarness(t *testing.T, binaryPath, dbBackendName string, ht.stopChainBackend = cleanUp // Connect our chainBackend to our miner. - t.Log("Connecting the miner with the chain backend...") - require.NoError(t, chainBackend.ConnectMiner(), "connect miner") + t.Logf("Connecting the miner at %v with the chain backend...", + miner.P2PAddress()) + + // Give the chain backend some time to fully start up, re-trying if any + // errors in connecting to the miner are encountered. + err := wait.NoError(func() error { + return chainBackend.ConnectMiner() + }, DefaultTimeout) + require.NoError(t, err, "connect miner") // Start the HarnessTest with the chainBackend and miner. ht.Start(chainBackend, miner) From 17b93db5cc4c6bdb6174249f44a7c9c9553d0706 Mon Sep 17 00:00:00 2001 From: Oliver Gugger Date: Mon, 18 Mar 2024 08:57:55 +0100 Subject: [PATCH 12/13] chainntnfs: fix race unit test issue Because we pass in the same config into multiple notifiers, we sometimes get a data race in the unit tests. By creating a copy of the config we avoid that. --- chainntnfs/test/test_interface.go | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/chainntnfs/test/test_interface.go b/chainntnfs/test/test_interface.go index 9a8a89879a..429b083a05 100644 --- a/chainntnfs/test/test_interface.go +++ b/chainntnfs/test/test_interface.go @@ -1953,9 +1953,10 @@ func TestInterfaces(t *testing.T, targetBackEnd string) { } case "btcd": + configCopy := rpcConfig newNotifier = func() (chainntnfs.TestChainNotifier, error) { return btcdnotify.New( - &rpcConfig, unittest.NetParams, + &configCopy, unittest.NetParams, hintCache, hintCache, blockCache, ) } From f6656d1daaed1b89d012e1e3d1a51e022d9aeddd Mon Sep 17 00:00:00 2001 From: Oliver Gugger Date: Mon, 18 Mar 2024 11:56:35 +0100 Subject: [PATCH 13/13] Travis+GitHub: replace Travis CI with GitHub MacOS 14 build --- .github/workflows/main.yml | 38 +++++++++++++++++++++++++++++++ .travis.yml | 46 -------------------------------------- 2 files changed, 38 insertions(+), 46 deletions(-) delete mode 100644 .travis.yml diff --git a/.github/workflows/main.yml b/.github/workflows/main.yml index 61fbeef589..bd9edc4740 100644 --- a/.github/workflows/main.yml +++ b/.github/workflows/main.yml @@ -328,6 +328,44 @@ jobs: path: logs-itest-windows.zip retention-days: 5 + ######################## + # run macOS integration test + ######################## + macos-integration-test: + name: run macOS itest + runs-on: macos-14 + if: '!contains(github.event.pull_request.labels.*.name, ''no-itest'')' + steps: + - name: git checkout + uses: actions/checkout@v3 + + - name: setup go ${{ env.GO_VERSION }} + uses: ./.github/actions/setup-go + with: + go-version: '${{ env.GO_VERSION }}' + key-prefix: integration-test + + - name: install bitcoind + run: | + wget https://bitcoincore.org/bin/bitcoin-core-${BITCOIN_VERSION}.0/bitcoin-${BITCOIN_VERSION}.0-arm64-apple-darwin.tar.gz + tar zxvf bitcoin-${BITCOIN_VERSION}.0-arm64-apple-darwin.tar.gz + mv bitcoin-${BITCOIN_VERSION}.0 /tmp/bitcoin + + - name: run itest + run: PATH=$PATH:/tmp/bitcoin/bin make itest-parallel backend=bitcoind + + - name: Zip log files on failure + if: ${{ failure() }} + timeout-minutes: 5 # timeout after 5 minute + run: 7z a logs-itest-macos.zip itest/**/*.log + + - name: Upload log files on failure + uses: actions/upload-artifact@v3 + if: ${{ failure() }} + with: + name: logs-itest-macos + path: logs-itest-macos.zip + retention-days: 5 ######################## # check pinned dependencies diff --git a/.travis.yml b/.travis.yml deleted file mode 100644 index b7c7734d94..0000000000 --- a/.travis.yml +++ /dev/null @@ -1,46 +0,0 @@ -language: go -cache: - directories: - - $GOCACHE - - $GOPATH/pkg/mod - - $GOPATH/src/github.com/btcsuite - - $GOPATH/src/github.com/golang - - $GOPATH/src/github.com/grpc-ecosystem - - $GOPATH/src/gopkg.in/alecthomas - - $GOPATH/src/google.golang.org - -# Remove Travis' default flag --depth=50 from the git clone command to make sure -# we have the whole git history, including the commit we lint against. -git: - depth: false - -go: - # If you change this value, please change it in the following files as well: - # /Dockerfile - # /dev.Dockerfile - # /make/builder.Dockerfile - # /.github/workflows/main.yml - # /.github/workflows/release.yml - - "1.21.0" - -env: - global: - - GOCACHE=$HOME/.go-build - - BITCOIN_VERSION="26" - -sudo: required - -jobs: - include: - - stage: Integration Test - name: Bitcoind Integration ARM - script: - - bash ./scripts/install_bitcoind.sh $BITCOIN_VERSION - - GOMEMLIMIT=1024MiB GOARM=7 GOARCH=arm GOOS=linux travis_wait 120 make itest-parallel backend=bitcoind tranches=8 - arch: arm64 - -after_failure: - - |- - LOG_FILES=$(find ./itest -name '*.log') - echo "Uploading to termbin.com..." && for f in $LOG_FILES; do echo -n $f; cat $f | nc termbin.com 9999 | xargs -r0 printf ' uploaded to %s'; done - echo "Uploading to file.io..." && tar -zcvO $LOG_FILES | curl -s -F 'file=@-;filename=logs.tar.gz' https://file.io | xargs -r0 printf 'logs.tar.gz uploaded to %s\n'