diff --git a/docs/release-notes/release-notes-0.20.0.md b/docs/release-notes/release-notes-0.20.0.md index 3ba247cb9..a3e76581e 100644 --- a/docs/release-notes/release-notes-0.20.0.md +++ b/docs/release-notes/release-notes-0.20.0.md @@ -125,6 +125,13 @@ circuit. The indices are only available for forwarding events saved after v0.20. ## Testing +* Previously, automatic peer bootstrapping was disabled for simnet, signet and + regtest networks even if the `--nobootstrap` flag was not set. This automatic + disabling has now been + [removed](https://github.com/lightningnetwork/lnd/pull/9967) meaning that any + test network scripts that rely on bootstrapping being disabled will need to + explicitly define the `--nobootstrap` flag. + ## Database ## Code Health diff --git a/itest/list_on_test.go b/itest/list_on_test.go index ec7f672e1..dedf4be43 100644 --- a/itest/list_on_test.go +++ b/itest/list_on_test.go @@ -707,6 +707,10 @@ var allTestCases = []*lntest.TestCase{ Name: "partially specified route blinded invoice", TestFunc: testPartiallySpecifiedBlindedPath, }, + { + Name: "peer bootstrapping", + TestFunc: testPeerBootstrapping, + }, } // appendPrefixed is used to add a prefix to each test name in the subtests diff --git a/itest/lnd_graph.go b/itest/lnd_graph.go new file mode 100644 index 000000000..206948227 --- /dev/null +++ b/itest/lnd_graph.go @@ -0,0 +1,77 @@ +package itest + +import ( + "time" + + "github.com/btcsuite/btcd/btcutil" + "github.com/lightningnetwork/lnd/lntest" + "github.com/lightningnetwork/lnd/lntest/wait" + "github.com/stretchr/testify/require" +) + +// testPeerBootstrapping tests that a node is able to use persisted node +// announcements to bootstrap its peer connections. This is done by +// connecting a node to a channel network so that it syncs the necessary gossip +// and then: +// 1. Asserting that on a restart where bootstrapping is _disabled_, the node +// does not connect to any peers. +// 2. Asserting that on a restart where bootstrapping is _enabled_, the node +// does connect to peers. +func testPeerBootstrapping(ht *lntest.HarnessTest) { + // 1) Set up the following node/channel network. + // Alice <- Bob <- Charlie + _, nodes := ht.CreateSimpleNetwork( + [][]string{nil, nil, nil}, lntest.OpenChannelParams{ + Amt: btcutil.Amount(100000), + }, + ) + carol, bob, alice := nodes[0], nodes[1], nodes[2] + + // Assert that they all know about each other and the channels. + ht.AssertNumEdges(alice, 2, false) + ht.AssertNumEdges(bob, 2, false) + ht.AssertNumEdges(carol, 2, false) + + // Spin up a new node, Dave. + dave := ht.NewNode("Dave", nil) + + // Explicitly assert that Dave was started with bootstrapping disabled. + require.False(ht, dave.Cfg.WithPeerBootstrap) + + // Assert that Dave's current view of the graph is empty. + ht.AssertNumEdges(dave, 0, false) + + // Now, connect Dave to Alice and wait for gossip sync to complete. + ht.EnsureConnected(dave, alice) + ht.AssertNumEdges(dave, 2, false) + + // Disconnect Dave from Alice and restart Dave. Since Alice and Dave + // did not have channels between them, the nodes should not reconnect + // to each other. + ht.DisconnectNodes(dave, alice) + ht.RestartNode(dave) + + // Dave still has bootstrapping disabled and so it should not connect + // to any peers. + err := wait.Invariant(func() bool { + peerResp := dave.RPC.ListPeers() + + return len(peerResp.Peers) == 0 + }, time.Second*5) + require.NoError(ht, err) + + // Dave should still know about the full graph though since it was + // synced previously from Alice. + ht.AssertNumEdges(dave, 2, false) + + // Restart Dave again but this time with bootstrapping enabled. + dave.Cfg.WithPeerBootstrap = true + ht.RestartNode(dave) + + // Show that Dave now does connect to some peers. The default minimum + // number of peers that will be connected to is 3, so we + // expect Dave to connect to all three nodes in the network. + ht.AssertConnected(dave, alice) + ht.AssertConnected(dave, bob) + ht.AssertConnected(dave, carol) +} diff --git a/lntest/node/config.go b/lntest/node/config.go index 694e26c3e..7f3b1df7f 100644 --- a/lntest/node/config.go +++ b/lntest/node/config.go @@ -146,8 +146,9 @@ type BaseNodeConfig struct { ReadMacPath string InvoiceMacPath string - SkipUnlock bool - Password []byte + SkipUnlock bool + Password []byte + WithPeerBootstrap bool P2PPort int RPCPort int @@ -285,7 +286,6 @@ func (cfg *BaseNodeConfig) GenArgs() []string { args = append(args, backendArgs...) nodeArgs := []string{ - "--nobootstrap", "--debuglevel=debug", "--bitcoin.defaultchanconfs=1", "--accept-keysend", @@ -326,6 +326,10 @@ func (cfg *BaseNodeConfig) GenArgs() []string { args = append(args, "--noseedbackup") } + if !cfg.WithPeerBootstrap { + args = append(args, "--nobootstrap") + } + switch cfg.DBBackend { case BackendEtcd: args = append(args, "--db.backend=etcd") diff --git a/server.go b/server.go index 47d1eaaf4..d8d1f4422 100644 --- a/server.go +++ b/server.go @@ -2644,7 +2644,7 @@ func (s *server) Start(ctx context.Context) error { // configure the set of active bootstrappers, and launch a // dedicated goroutine to maintain a set of persistent // connections. - if shouldPeerBootstrap(s.cfg) { + if !s.cfg.NoNetBootstrap { bootstrappers, err := initNetworkBootstrappers(s) if err != nil { startErr = err @@ -5378,20 +5378,6 @@ func newSweepPkScriptGen( } } -// shouldPeerBootstrap returns true if we should attempt to perform peer -// bootstrapping to actively seek our peers using the set of active network -// bootstrappers. -func shouldPeerBootstrap(cfg *Config) bool { - isSimnet := cfg.Bitcoin.SimNet - isSignet := cfg.Bitcoin.SigNet - isRegtest := cfg.Bitcoin.RegTest - isDevNetwork := isSimnet || isSignet || isRegtest - - // TODO(yy): remove the check on simnet/regtest such that the itest is - // covering the bootstrapping process. - return !cfg.NoNetBootstrap && !isDevNetwork -} - // fetchClosedChannelSCIDs returns a set of SCIDs that have their force closing // finished. func (s *server) fetchClosedChannelSCIDs() map[lnwire.ShortChannelID]struct{} { diff --git a/server_test.go b/server_test.go deleted file mode 100644 index 78866d598..000000000 --- a/server_test.go +++ /dev/null @@ -1,82 +0,0 @@ -package lnd - -import ( - "testing" - - "github.com/lightningnetwork/lnd/lncfg" -) - -// TestShouldPeerBootstrap tests that we properly skip network bootstrap for -// the developer networks, and also if bootstrapping is explicitly disabled. -func TestShouldPeerBootstrap(t *testing.T) { - t.Parallel() - - testCases := []struct { - cfg *Config - shouldBoostrap bool - }{ - // Simnet active, no bootstrap. - { - cfg: &Config{ - Bitcoin: &lncfg.Chain{ - SimNet: true, - }, - }, - }, - - // Regtest active, no bootstrap. - { - cfg: &Config{ - Bitcoin: &lncfg.Chain{ - RegTest: true, - }, - }, - }, - - // Signet active, no bootstrap. - { - cfg: &Config{ - Bitcoin: &lncfg.Chain{ - SigNet: true, - }, - }, - }, - - // Mainnet active, but bootstrap disabled, no bootstrap. - { - cfg: &Config{ - Bitcoin: &lncfg.Chain{ - MainNet: true, - }, - NoNetBootstrap: true, - }, - }, - - // Mainnet active, should bootstrap. - { - cfg: &Config{ - Bitcoin: &lncfg.Chain{ - MainNet: true, - }, - }, - shouldBoostrap: true, - }, - - // Testnet active, should bootstrap. - { - cfg: &Config{ - Bitcoin: &lncfg.Chain{ - TestNet3: true, - }, - }, - shouldBoostrap: true, - }, - } - for i, testCase := range testCases { - bootstrapped := shouldPeerBootstrap(testCase.cfg) - if bootstrapped != testCase.shouldBoostrap { - t.Fatalf("#%v: expected bootstrap=%v, got bootstrap=%v", - i, testCase.shouldBoostrap, bootstrapped) - } - } -}