From b1eaeb6ac62c6de9becf7a919034759a024d535b Mon Sep 17 00:00:00 2001 From: Olaoluwa Osuntokun Date: Tue, 19 Sep 2023 18:31:22 -0700 Subject: [PATCH 1/3] build: prep for Go 1.22 by using GOEXPERIMENT=loopvar for tests In this commit, [we prep for some upcoming changes in Go](https://go.dev/blog/loopvar-preview) by running our tests with `GOEXPERIMENT=loopvar`. This changes the loop semantics to fix a common bug where a scoping issue causes a variable to be re-used, which can cause unintended bugs down the line. If everything passes with this flag on, then we can keep it on, and also rest a bit easier knowing that the compiler will fix a class of bugs that would previously pop up for us. --- Makefile | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Makefile b/Makefile index a6ca526ba..6c526f67b 100644 --- a/Makefile +++ b/Makefile @@ -23,7 +23,7 @@ COMMIT := $(shell git describe --tags --dirty) GOBUILD := go build -v GOINSTALL := go install -v -GOTEST := go test +GOTEST := GOEXPERIMENT=loopvar go test GOFILES_NOVENDOR = $(shell find . -type f -name '*.go' -not -path "./vendor/*" -not -name "*pb.go" -not -name "*pb.gw.go" -not -name "*.pb.json.go") From 206ad69b75404c711678670b07b9fe91b9842885 Mon Sep 17 00:00:00 2001 From: Olaoluwa Osuntokun Date: Wed, 4 Oct 2023 15:36:52 -0700 Subject: [PATCH 2/3] discovery: fix incorrect test, masked by loop scoping bug When we first go to boot up the syncer, when we're in the active phase, after we do the historical sync, we'll send a timestamp message that we want everything, then transition to the passive mode. The test didn't account for this extra message before, as the last test was being re-used here (ran in parallel). We fix this by asserting that the first expected message is sent, then also the follow up messages as well. --- discovery/chan_series.go | 2 ++ discovery/syncer_test.go | 27 ++++++++++++++++++++++----- 2 files changed, 24 insertions(+), 5 deletions(-) diff --git a/discovery/chan_series.go b/discovery/chan_series.go index 0fe819d4e..18f806316 100644 --- a/discovery/chan_series.go +++ b/discovery/chan_series.go @@ -163,6 +163,8 @@ func (c *ChanSeries) UpdatesInHorizon(chain chainhash.Hash, return nil, err } for _, nodeAnn := range nodeAnnsInHorizon { + nodeAnn := nodeAnn + // Ensure we only forward nodes that are publicly advertised to // prevent leaking information about nodes. isNodePublic, err := c.graph.IsPublicNode(nodeAnn.PubKeyBytes) diff --git a/discovery/syncer_test.go b/discovery/syncer_test.go index 70920f9f4..a7b514db8 100644 --- a/discovery/syncer_test.go +++ b/discovery/syncer_test.go @@ -2134,18 +2134,33 @@ func TestGossipSyncerSyncTransitions(t *testing.T) { name: "active to passive", entrySyncType: ActiveSync, finalSyncType: PassiveSync, - assert: func(t *testing.T, msgChan chan []lnwire.Message, + assert: func(t *testing.T, mChan chan []lnwire.Message, g *GossipSyncer) { + // When we first boot up, we expect that we + // send out a message that indicates we want + // all the updates from here on. + firstTimestamp := uint32(time.Now().Unix()) + assertMsgSent( + t, mChan, &lnwire.GossipTimestampRange{ + FirstTimestamp: firstTimestamp, + TimestampRange: math.MaxUint32, + }, + ) + // When transitioning from active to passive, we // should expect to see a new local update // horizon sent to the remote peer indicating // that it would not like to receive any future // updates. - assertMsgSent(t, msgChan, &lnwire.GossipTimestampRange{ - FirstTimestamp: uint32(zeroTimestamp.Unix()), - TimestampRange: 0, - }) + assertMsgSent( + t, mChan, &lnwire.GossipTimestampRange{ + FirstTimestamp: uint32( + zeroTimestamp.Unix(), + ), + TimestampRange: 0, + }, + ) syncState := g.syncState() if syncState != chansSynced { @@ -2182,6 +2197,8 @@ func TestGossipSyncerSyncTransitions(t *testing.T) { } for _, test := range tests { + test := test + t.Run(test.name, func(t *testing.T) { t.Parallel() From 63a94ad9873cf6837227d29d17b8a4e3d8624e98 Mon Sep 17 00:00:00 2001 From: Olaoluwa Osuntokun Date: Wed, 4 Oct 2023 15:41:19 -0700 Subject: [PATCH 3/3] build: build+install using GOEXPERIMENT=loopvar We also update the release script to do the same as well. --- Makefile | 14 +++++++++++--- scripts/release.sh | 6 +++--- 2 files changed, 14 insertions(+), 6 deletions(-) diff --git a/Makefile b/Makefile index 6c526f67b..0ed697d4b 100644 --- a/Makefile +++ b/Makefile @@ -21,9 +21,17 @@ ANDROID_BUILD := $(ANDROID_BUILD_DIR)/Lndmobile.aar COMMIT := $(shell git describe --tags --dirty) -GOBUILD := go build -v -GOINSTALL := go install -v -GOTEST := GOEXPERIMENT=loopvar go test +GO_VERSION := $(shell go version | sed -nre 's/^[^0-9]*(([0-9]+\.)*[0-9]+).*/\1/p') +GO_VERSION_MINOR := $(shell echo $(GO_VERSION) | cut -d. -f2) + +LOOPVARFIX := +ifeq ($(shell expr $(GO_VERSION_MINOR) \>= 21), 1) + LOOPVARFIX := GOEXPERIMENT=loopvar +endif + +GOBUILD := $(LOOPVARFIX) go build -v +GOINSTALL := $(LOOPVARFIX) go install -v +GOTEST := $(LOOPVARFIX) go test GOFILES_NOVENDOR = $(shell find . -type f -name '*.go' -not -path "./vendor/*" -not -name "*pb.go" -not -name "*pb.gw.go" -not -name "*.pb.json.go") diff --git a/scripts/release.sh b/scripts/release.sh index 930d680e3..0f6b44cff 100755 --- a/scripts/release.sh +++ b/scripts/release.sh @@ -98,7 +98,7 @@ function check_tag_correct() { fi # Build lnd to extract version. - go build ${PKG}/cmd/lnd + env GOEXPERIMENT=loopvar go build ${PKG}/cmd/lnd # Extract version command output. lnd_version_output=$(./lnd --version) @@ -177,8 +177,8 @@ function build_release() { pushd "${dir}" green " - Building: ${os} ${arch} ${arm} with build tags '${buildtags}'" - env CGO_ENABLED=0 GOOS=$os GOARCH=$arch GOARM=$arm go build -v -trimpath -ldflags="${ldflags}" -tags="${buildtags}" ${PKG}/cmd/lnd - env CGO_ENABLED=0 GOOS=$os GOARCH=$arch GOARM=$arm go build -v -trimpath -ldflags="${ldflags}" -tags="${buildtags}" ${PKG}/cmd/lncli + env GOEXPERIMENT=loopvar CGO_ENABLED=0 GOOS=$os GOARCH=$arch GOARM=$arm go build -v -trimpath -ldflags="${ldflags}" -tags="${buildtags}" ${PKG}/cmd/lnd + env GOEXPERIMENT=loopvar CGO_ENABLED=0 GOOS=$os GOARCH=$arch GOARM=$arm go build -v -trimpath -ldflags="${ldflags}" -tags="${buildtags}" ${PKG}/cmd/lncli popd # Add the hashes for the individual binaries as well for easy verification