Merge pull request #1881 from halseth/neutrino-integration-tests

Neutrino integration tests

.travis.yml

@@ -17,6 +17,7 @@ env:
   matrix:
     - RACE=true
     - ITEST=true
+    - NEUTRINO_ITEST=true
     - COVER=true 
 
 sudo: required
@@ -28,9 +29,12 @@ script:
   # Run unit tests with race condition detector.
   - 'if [ "$RACE" = true ]; then make travis-race ; fi'
 
-  # Run integration tests.
+  # Run btcd integration tests.
   - 'if [ "$ITEST" = true ]; then make travis-itest; fi'
 
+  # Run neutrino integration tests.
+  - 'if [ "$NEUTRINO_ITEST" = true ]; then make travis-itest backend=neutrino; fi'
+
   # Run unit tests and generate coverage report.
   - 'if [ "$COVER" = true ]; then make travis-cover; fi'
 

Makefile

@@ -115,7 +115,7 @@ scratch: build
 check: unit itest
 
 itest-only:
-	@$(call print, "Running integration tests.")
+	@$(call print, "Running integration tests with ${backend} backend.")
 	$(ITEST)
 
 itest: btcd build-itest itest-only
@@ -149,7 +149,7 @@ travis-itest: lint itest
 # =============
 
 flakehunter: build-itest
-	@$(call print, "Flake hunting integration tests.")
+	@$(call print, "Flake hunting ${backend} integration tests.")
 	while [ $$? -eq 0 ]; do $(ITEST); done
 
 flake-unit:

lnd_test.go

@@ -68,6 +68,12 @@ func newHarnessTest(t *testing.T) *harnessTest {
 	return &harnessTest{t, nil}
 }
 
+// Skipf calls the underlying testing.T's Skip method, causing the current test
+// to be skipped.
+func (h *harnessTest) Skipf(format string, args ...interface{}) {
+	h.t.Skipf(format, args...)
+}
+
 // Fatalf causes the current active test case to fail with a fatal error. All
 // integration tests should mark test failures solely with this method due to
 // the error stack traces it produces.
@@ -713,7 +719,7 @@ func getChanInfo(ctx context.Context, node *lntest.HarnessNode) (
 	}
 	if len(channelInfo.Channels) != 1 {
 		return nil, fmt.Errorf("node should only have a single "+
-			"channel, instead he has %v", len(channelInfo.Channels))
+			"channel, instead it has %v", len(channelInfo.Channels))
 	}
 
 	return channelInfo.Channels[0], nil
@@ -972,8 +978,8 @@ func testBasicChannelFunding(net *lntest.NetworkHarness, t *harnessTest) {
 	closeChannelAndAssert(ctxt, t, net, net.Alice, chanPoint, false)
 }
 
-// testUnconfirmedChannelFunding tests that unconfirmed outputs that pay to us
+// testUnconfirmedChannelFunding tests that our unconfirmed change outputs can
-// can be used to fund channels.
+// be used to fund channels.
 func testUnconfirmedChannelFunding(net *lntest.NetworkHarness, t *harnessTest) {
 	ctxb := context.Background()
 
@@ -989,13 +995,34 @@ func testUnconfirmedChannelFunding(net *lntest.NetworkHarness, t *harnessTest) {
 	}
 	defer shutdownAndAssert(net, t, carol)
 
-	// We'll send her some funds that should not confirm.
+	// We'll send her some confirmed funds.
 	ctxt, _ := context.WithTimeout(ctxb, defaultTimeout)
-	err = net.SendCoinsUnconfirmed(ctxt, 2*chanAmt, carol)
+	err = net.SendCoins(ctxt, 2*chanAmt, carol)
 	if err != nil {
 		t.Fatalf("unable to send coins to carol: %v", err)
 	}
 
+	// Now let Carol send some funds to herself, making a unconfirmed
+	// change output.
+	addrReq := &lnrpc.NewAddressRequest{
+		Type: lnrpc.AddressType_WITNESS_PUBKEY_HASH,
+	}
+	ctxt, _ = context.WithTimeout(ctxb, defaultTimeout)
+	resp, err := carol.NewAddress(ctxt, addrReq)
+	if err != nil {
+		t.Fatalf("unable to get new address: %v", err)
+	}
+
+	sendReq := &lnrpc.SendCoinsRequest{
+		Addr:   resp.Address,
+		Amount: int64(chanAmt) / 5,
+	}
+	ctxt, _ = context.WithTimeout(ctxb, defaultTimeout)
+	_, err = carol.SendCoins(ctxt, sendReq)
+	if err != nil {
+		t.Fatalf("unable to send coins: %v", err)
+	}
+
 	// Make sure the unconfirmed tx is seen in the mempool.
 	_, err = waitForTxInMempool(net.Miner.Node, minerMempoolTimeout)
 	if err != nil {
@@ -1760,10 +1787,16 @@ func assertMinerBlockHeightDelta(t *harnessTest,
 // channel where the funding tx gets reorged out, the channel will no
 // longer be present in the node's routing table.
 func testOpenChannelAfterReorg(net *lntest.NetworkHarness, t *harnessTest) {
+	// Skip test for neutrino, as we cannot disconnect the miner at will.
+	// TODO(halseth): remove when either can disconnect at will, or restart
+	// node with connection to new miner.
+	if net.BackendCfg.Name() == "neutrino" {
+		t.Skipf("skipping reorg test for neutrino backend")
+	}
+
 	var (
 		ctxb = context.Background()
 		temp = "temp"
-		perm = "perm"
 	)
 
 	// Set up a new miner that we can use to cause a reorg.
@@ -1901,9 +1934,7 @@ func testOpenChannelAfterReorg(net *lntest.NetworkHarness, t *harnessTest) {
 	// Now we disconnect Alice's chain backend from the original miner, and
 	// connect the two miners together. Since the temporary miner knows
 	// about a longer chain, both miners should sync to that chain.
-	err = net.Miner.Node.Node(
+	err = net.BackendCfg.DisconnectMiner()
-		btcjson.NRemove, net.BackendCfg.P2PAddr(), &perm,
-	)
 	if err != nil {
 		t.Fatalf("unable to remove node: %v", err)
 	}
@@ -1934,9 +1965,7 @@ func testOpenChannelAfterReorg(net *lntest.NetworkHarness, t *harnessTest) {
 		t.Fatalf("unable to remove node: %v", err)
 	}
 
-	err = net.Miner.Node.Node(
+	err = net.BackendCfg.ConnectMiner()
-		btcjson.NConnect, net.BackendCfg.P2PAddr(), &perm,
-	)
 	if err != nil {
 		t.Fatalf("unable to remove node: %v", err)
 	}
@@ -13316,13 +13345,6 @@ var testsCases = []*testCase{
 func TestLightningNetworkDaemon(t *testing.T) {
 	ht := newHarnessTest(t)
 
-	// Start a btcd chain backend.
-	chainBackend, cleanUp, err := lntest.NewBtcdBackend()
-	if err != nil {
-		ht.Fatalf("unable to start btcd: %v", err)
-	}
-	defer cleanUp()
-
 	// Declare the network harness here to gain access to its
 	// 'OnTxAccepted' call back.
 	var lndHarness *lntest.NetworkHarness
@@ -13343,7 +13365,6 @@ func TestLightningNetworkDaemon(t *testing.T) {
 		"--debuglevel=debug",
 		"--logdir=" + minerLogDir,
 		"--trickleinterval=100ms",
-		"--connect=" + chainBackend.P2PAddr(),
 	}
 	handlers := &rpcclient.NotificationHandlers{
 		OnTxAccepted: func(hash *chainhash.Hash, amt btcutil.Amount) {
@@ -13373,6 +13394,13 @@ func TestLightningNetworkDaemon(t *testing.T) {
 		}
 	}()
 
+	// Start a chain backend.
+	chainBackend, cleanUp, err := lntest.NewBackend(miner.P2PAddress())
+	if err != nil {
+		ht.Fatalf("unable to start backend: %v", err)
+	}
+	defer cleanUp()
+
 	if err := miner.SetUp(true, 50); err != nil {
 		ht.Fatalf("unable to set up mining node: %v", err)
 	}

lntest/btcd.go

@@ -1,3 +1,5 @@
+// +build btcd
+
 package lntest
 
 import (
@@ -5,6 +7,7 @@ import (
 	"fmt"
 	"os"
 
+	"github.com/btcsuite/btcd/btcjson"
 	"github.com/btcsuite/btcd/chaincfg"
 	"github.com/btcsuite/btcd/integration/rpctest"
 	"github.com/btcsuite/btcd/rpcclient"
@@ -13,14 +16,23 @@ 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
+// btcd Node API.
+//
+// NOTE: Cannot be const, since the node API expects a reference.
+var perm = "perm"
+
 // BtcdBackendConfig is an implementation of the BackendConfig interface
 // backed by a btcd node.
 type BtcdBackendConfig struct {
 	// rpcConfig houses the connection config to the backing btcd instance.
 	rpcConfig rpcclient.ConnConfig
 
-	// p2pAddress is the p2p address of the btcd instance.
+	// harness is the backing btcd instance.
-	p2pAddress string
+	harness *rpctest.Harness
+
+	// minerAddr is the p2p address of the miner to connect to.
+	minerAddr string
 }
 
 // GenArgs returns the arguments needed to be passed to LND at startup for
@@ -37,21 +49,32 @@ func (b BtcdBackendConfig) GenArgs() []string {
 	return args
 }
 
-// P2PAddr returns the address of this node to be used when connection over the
+// ConnectMiner is called to establish a connection to the test miner.
-// Bitcoin P2P network.
+func (b BtcdBackendConfig) ConnectMiner() error {
-func (b BtcdBackendConfig) P2PAddr() string {
+	return b.harness.Node.Node(btcjson.NConnect, b.minerAddr, &perm)
-	return b.p2pAddress
 }
 
-// NewBtcdBackend starts a new rpctest.Harness and returns a BtcdBackendConfig
+// DisconnectMiner is called to disconnect the miner.
-// for that node.
+func (b BtcdBackendConfig) DisconnectMiner() error {
-func NewBtcdBackend() (*BtcdBackendConfig, func(), error) {
+	return b.harness.Node.Node(btcjson.NRemove, b.minerAddr, &perm)
+}
+
+// Name returns the name of the backend type.
+func (b BtcdBackendConfig) Name() string {
+	return "btcd"
+}
+
+// NewBackend starts a new rpctest.Harness and returns a BtcdBackendConfig for
+// that node. miner should be set to the P2P address of the miner to connect
+// to.
+func NewBackend(miner string) (*BtcdBackendConfig, func(), error) {
 	args := []string{
 		"--rejectnonstd",
 		"--txindex",
 		"--trickleinterval=100ms",
 		"--debuglevel=debug",
 		"--logdir=" + logDir,
+		"--connect=" + miner,
 	}
 	netParams := &chaincfg.SimNetParams
 	chainBackend, err := rpctest.New(netParams, nil, args)
@@ -65,7 +88,8 @@ func NewBtcdBackend() (*BtcdBackendConfig, func(), error) {
 
 	bd := &BtcdBackendConfig{
 		rpcConfig: chainBackend.RPCConfig(),
-		p2pAddress: chainBackend.P2PAddress(),
+		harness:   chainBackend,
+		minerAddr: miner,
 	}
 
 	cleanUp := func() {

lntest/harness.go

@@ -1314,6 +1314,12 @@ func (n *NetworkHarness) sendCoins(ctx context.Context, amt btcutil.Amount,
 	// Now, wait for ListUnspent to show the unconfirmed transaction
 	// containing the correct pkscript.
 	err = WaitNoError(func() error {
+		// Since neutrino doesn't support unconfirmed outputs, skip
+		// this check.
+		if target.cfg.BackendCfg.Name() == "neutrino" {
+			return nil
+		}
+
 		req := &lnrpc.ListUnspentRequest{}
 		resp, err := target.ListUnspent(ctx, req)
 		if err != nil {

lntest/neutrino.go

@@ -0,0 +1,45 @@
+// +build neutrino
+
+package lntest
+
+import "fmt"
+
+// NeutrinoBackendConfig is an implementation of the BackendConfig interface
+// backed by a neutrino node.
+type NeutrinoBackendConfig struct {
+	minerAddr string
+}
+
+// GenArgs returns the arguments needed to be passed to LND at startup for
+// using this node as a chain backend.
+func (b NeutrinoBackendConfig) GenArgs() []string {
+	var args []string
+	args = append(args, "--bitcoin.node=neutrino")
+	args = append(args, "--neutrino.connect="+b.minerAddr)
+	return args
+}
+
+// ConnectMiner is called to establish a connection to the test miner.
+func (b NeutrinoBackendConfig) ConnectMiner() error {
+	return fmt.Errorf("unimplemented")
+}
+
+// DisconnectMiner is called to disconnect the miner.
+func (b NeutrinoBackendConfig) DisconnectMiner() error {
+	return fmt.Errorf("unimplemented")
+}
+
+// Name returns the name of the backend type.
+func (b NeutrinoBackendConfig) Name() string {
+	return "neutrino"
+}
+
+// NewBackend starts and returns a NeutrinoBackendConfig for the node.
+func NewBackend(miner string) (*NeutrinoBackendConfig, func(), error) {
+	bd := &NeutrinoBackendConfig{
+		minerAddr: miner,
+	}
+
+	cleanUp := func() {}
+	return bd, cleanUp, nil
+}

lntest/node.go

@@ -99,9 +99,14 @@ type BackendConfig interface {
 	// for using this node as a chain backend.
 	GenArgs() []string
 
-	// P2PAddr returns the address of this node to be used when connection
+	// ConnectMiner is called to establish a connection to the test miner.
-	// over the Bitcoin P2P network.
+	ConnectMiner() error
-	P2PAddr() string
+
+	// DisconnectMiner is called to bitconneeeect the miner.
+	DisconnectMiner() error
+
+	// Name returns the name of the backend type.
+	Name() string
 }
 
 type nodeConfig struct {

make/testing_flags.mk

@@ -35,7 +35,7 @@ endif
 ifneq ($(timeout),)
 TEST_FLAGS += -test.timeout=$(timeout)
 else
-TEST_FLAGS += -test.timeout=30m
+TEST_FLAGS += -test.timeout=40m
 endif
 
 # UNIT_TARGTED is undefined iff a specific package and/or unit test case is
@@ -57,4 +57,12 @@ endif
 
 # Construct the integration test command with the added build flags.
 ITEST_TAGS := $(DEV_TAGS) rpctest chainrpc walletrpc signrpc invoicesrpc autopilotrpc routerrpc
+
+# Default to btcd backend if not set.
+ifneq ($(backend),)
+ITEST_TAGS += ${backend}
+else
+ITEST_TAGS += btcd
+endif
+
 ITEST := rm output*.log; date; $(GOTEST) -tags="$(ITEST_TAGS)" $(TEST_FLAGS) -logoutput

rpcserver.go

@@ -2099,6 +2099,8 @@ func (r *rpcServer) WalletBalance(ctx context.Context,
 	}
 
 	// Get confirmed balance, from txs that have >= 1 confirmations.
+	// TODO(halseth): get both unconfirmed and confirmed balance in one
+	// call, as this is racy.
 	confirmedBal, err := r.server.cc.wallet.ConfirmedBalance(1)
 	if err != nil {
 		return nil, err
@@ -2107,7 +2109,8 @@ func (r *rpcServer) WalletBalance(ctx context.Context,
 	// Get unconfirmed balance, from txs with 0 confirmations.
 	unconfirmedBal := totalBal - confirmedBal
 
-	rpcsLog.Debugf("[walletbalance] Total balance=%v", totalBal)
+	rpcsLog.Debugf("[walletbalance] Total balance=%v (confirmed=%v, "+
+		"unconfirmed=%v)", totalBal, confirmedBal, unconfirmedBal)
 
 	return &lnrpc.WalletBalanceResponse{
 		TotalBalance:       int64(totalBal),