From 38862d3b910d9de71ac72db9798412a16f3c3564 Mon Sep 17 00:00:00 2001 From: Elle Mouton Date: Sat, 16 Nov 2024 09:43:18 +0200 Subject: [PATCH 1/4] server: remove `shouldPeerBootstrap` We already set `nobootstrap` in the default node flags for itest nodes, so we can remove this check now. This will allow us to later test bootstrapping in an itest. NOTE that with this change, any signet/simnet/regtest network users will now need to explicitly add the `--nobootstrap` flag if they want to prevent automatic bootstrapping. This warning is added to the release notes later on. --- server.go | 16 +--------- server_test.go | 82 -------------------------------------------------- 2 files changed, 1 insertion(+), 97 deletions(-) delete mode 100644 server_test.go 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) - } - } -} From 63fbd5fbcb5cbcf8107fd84259c4422b9aa6f724 Mon Sep 17 00:00:00 2001 From: Elle Mouton Date: Mon, 18 Nov 2024 13:42:18 +0200 Subject: [PATCH 2/4] lntest: make nobootstrap configurable This will allow us to test bootstrapping in an itest. --- lntest/node/config.go | 10 +++++++--- 1 file changed, 7 insertions(+), 3 deletions(-) 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") From 4d766c457fa509cb5d93473a71a6f88b7be11b37 Mon Sep 17 00:00:00 2001 From: Elle Mouton Date: Fri, 20 Jun 2025 10:32:23 +0200 Subject: [PATCH 3/4] itest: add peer bootstrapping itest Test the automatic peer bootstrapping logic via an itest. --- itest/list_on_test.go | 4 +++ itest/lnd_graph.go | 77 +++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 81 insertions(+) create mode 100644 itest/lnd_graph.go 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) +} From 75a353146d43824282a54f34fe8155c0eb3b535b Mon Sep 17 00:00:00 2001 From: Elle Mouton Date: Fri, 20 Jun 2025 10:32:46 +0200 Subject: [PATCH 4/4] docs: update release notes Warn users of simnet/signet/regtest scripts that they will now need to explicitly specify the `--nobootstrap` flag to turn bootstrapping off. --- docs/release-notes/release-notes-0.20.0.md | 7 +++++++ 1 file changed, 7 insertions(+) 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