A thread safe way to take a free port

common/network/port.go

@@ -0,0 +1,36 @@
+package network
+
+import (
+	"errors"
+	"fmt"
+	"net"
+)
+
+const (
+	maxPortCheck = 100
+
+	emptyPort = "127.0.0.1:0"
+)
+
+var (
+	ErrCantFindAPort = errors.New("no available port found")
+)
+
+// FindAvailablePort returns the an available port
+func FindAvailablePort() (int, net.Listener, error) {
+	var (
+		listener net.Listener
+		err      error
+	)
+
+	for i := uint(0); i < maxPortCheck; i++ {
+		listener, err = net.Listen("tcp", emptyPort)
+		if err != nil {
+			continue
+		}
+
+		return listener.Addr().(*net.TCPAddr).Port, listener, nil
+	}
+
+	return 0, nil, fmt.Errorf("%w: %s", ErrCantFindAPort, err)
+}

consensus/bor/heimdall/client_test.go

@@ -9,11 +9,11 @@ import (
 	"net"
 	"net/http"
 	"sync"
-	"sync/atomic"
 	"testing"
 	"time"
 
 	"github.com/ethereum/go-ethereum/common"
+	"github.com/ethereum/go-ethereum/common/network"
 	"github.com/ethereum/go-ethereum/consensus/bor/heimdall/checkpoint"
 
 	"github.com/stretchr/testify/require"
@@ -34,7 +34,7 @@ func (h *HttpHandlerFake) GetCheckpointHandler() http.HandlerFunc {
 	}
 }
 
-func CreateMockHeimdallServer(wg *sync.WaitGroup, port int32, handler *HttpHandlerFake) (*http.Server, error) {
+func CreateMockHeimdallServer(wg *sync.WaitGroup, port int, listener net.Listener, handler *HttpHandlerFake) (*http.Server, error) {
 	// Create a new server mux
 	mux := http.NewServeMux()
 
@@ -51,6 +51,12 @@ func CreateMockHeimdallServer(wg *sync.WaitGroup, port int32, handler *HttpHandl
 		Handler: mux,
 	}
 
+	// Close the listener using the port and immediately consume it below
+	err := listener.Close()
+	if err != nil {
+		return nil, err
+	}
+
 	go func() {
 		defer wg.Done()
 
@@ -93,12 +99,12 @@ func TestFetchCheckpointFromMockHeimdall(t *testing.T) {
 		}
 	}
 
-	// Fetch available port starting from 50000
-	port, err := findAvailablePort(50000, 0)
+	// Fetch available port
+	port, listener, err := network.FindAvailablePort()
 	require.NoError(t, err, "expect no error in finding available port")
 
 	// Create mock heimdall server and pass handler instance for setting up the routes
-	srv, err := CreateMockHeimdallServer(wg, port, handler)
+	srv, err := CreateMockHeimdallServer(wg, port, listener, handler)
 	require.NoError(t, err, "expect no error in starting mock heimdall server")
 
 	// Create a new heimdall client and use same port for connection
@@ -150,12 +156,12 @@ func TestFetchShutdown(t *testing.T) {
 		}
 	}
 
-	// Fetch available port starting from 50000
-	port, err := findAvailablePort(50000, 0)
+	// Fetch available port
+	port, listener, err := network.FindAvailablePort()
 	require.NoError(t, err, "expect no error in finding available port")
 
 	// Create mock heimdall server and pass handler instance for setting up the routes
-	srv, err := CreateMockHeimdallServer(wg, port, handler)
+	srv, err := CreateMockHeimdallServer(wg, port, listener, handler)
 	require.NoError(t, err, "expect no error in starting mock heimdall server")
 
 	// Create a new heimdall client and use same port for connection
@@ -216,26 +222,6 @@ func TestFetchShutdown(t *testing.T) {
 	wg.Wait()
 }
 
-// findAvailablePort returns the next available port starting from `from`
-func findAvailablePort(from int32, count int32) (int32, error) {
-	if count == maxPortCheck {
-		return 0, fmt.Errorf("no available port found")
-	}
-
-	port := atomic.AddInt32(&from, 1)
-	addr := fmt.Sprintf("localhost:%d", port)
-
-	count++
-
-	lis, err := net.Listen("tcp", addr)
-	if err == nil {
-		lis.Close()
-		return port, nil
-	} else {
-		return findAvailablePort(from, count)
-	}
-}
-
 // TestContext includes bunch of simple tests to verify the working of timeout
 // based context and cancellation.
 func TestContext(t *testing.T) {

consensus/bor/heimdallgrpc/state_sync.go

@@ -3,10 +3,10 @@ package heimdallgrpc
 import (
 	"context"
 
+	proto "github.com/maticnetwork/polyproto/heimdall"
+
 	"github.com/ethereum/go-ethereum/common"
 	"github.com/ethereum/go-ethereum/consensus/bor/clerk"
-
-	proto "github.com/maticnetwork/polyproto/heimdall"
 )
 
 func (h *HeimdallGRPCClient) StateSyncEvents(ctx context.Context, fromID uint64, to int64) ([]*clerk.EventRecordWithTime, error) {
@@ -23,8 +23,10 @@ func (h *HeimdallGRPCClient) StateSyncEvents(ctx context.Context, fromID uint64,
 		return nil, err
 	}
 
+	var events *proto.StateSyncEventsResponse
+
 	for {
-		events, err := res.Recv()
+		events, err = res.Recv()
 		if err != nil {
 			break
 		}
@@ -45,5 +47,5 @@ func (h *HeimdallGRPCClient) StateSyncEvents(ctx context.Context, fromID uint64,
 		}
 	}
 
-	return eventRecords, nil
+	return eventRecords, err
 }

eth/downloader/downloader.go

@@ -207,7 +207,7 @@ type BlockChain interface {
 }
 
 // New creates a new downloader to fetch hashes and blocks from remote peers.
-//nolint: staticcheck
+// nolint: staticcheck
 func New(checkpoint uint64, stateDb ethdb.Database, mux *event.TypeMux, chain BlockChain, lightchain LightChain, dropPeer peerDropFn, success func(), whitelistService ethereum.ChainValidator) *Downloader {
 	if lightchain == nil {
 		lightchain = chain
@@ -729,9 +729,11 @@ func (d *Downloader) fetchHead(p *peerConnection) (head *types.Header, pivot *ty
 // calculateRequestSpan calculates what headers to request from a peer when trying to determine the
 // common ancestor.
 // It returns parameters to be used for peer.RequestHeadersByNumber:
-//  from - starting block number
-//  count - number of headers to request
-//  skip - number of headers to skip
+//
+//	from - starting block number
+//	count - number of headers to request
+//	skip - number of headers to skip
+//
 // and also returns 'max', the last block which is expected to be returned by the remote peers,
 // given the (from,count,skip)
 func calculateRequestSpan(remoteHeight, localHeight uint64) (int64, int, int, uint64) {

eth/filters/test_backend.go

@@ -48,10 +48,14 @@ func (b *TestBackend) GetBorBlockReceipt(ctx context.Context, hash common.Hash)
 
 func (b *TestBackend) GetBorBlockLogs(ctx context.Context, hash common.Hash) ([]*types.Log, error) {
 	receipt, err := b.GetBorBlockReceipt(ctx, hash)
-	if receipt == nil || err != nil {
+	if receipt == nil {
 		return []*types.Log{}, nil
 	}
 
+	if err != nil {
+		return []*types.Log{}, err
+	}
+
 	return receipt.Logs, nil
 }
 

internal/cli/debug_test.go

@@ -13,7 +13,7 @@ import (
 	"github.com/ethereum/go-ethereum/internal/cli/server"
 )
 
-var currentDir string = ""
+var currentDir string
 
 func TestCommand_DebugBlock(t *testing.T) {
 	t.Parallel()

internal/cli/server/command.go

@@ -72,7 +72,7 @@ func (c *Command) extractFlags(args []string) error {
 	}
 	// read if config file is provided, this will overwrite the cli flags, if provided
 	if c.configFile != "" {
-		log.Warn("Config File provided, this will overwrite the cli flags.", "configFile:", c.configFile)
+		log.Warn("Config File provided, this will overwrite the cli flags", "path", c.configFile)
 		cfg, err := readConfigFile(c.configFile)
 		if err != nil {
 			c.UI.Error(err.Error())
@@ -101,7 +101,7 @@ func (c *Command) Run(args []string) int {
 		return 1
 	}
 
-	srv, err := NewServer(c.config)
+	srv, err := NewServer(c.config, WithGRPCAddress())
 	if err != nil {
 		c.UI.Error(err.Error())
 		return 1

internal/cli/server/config_legacy.go

@@ -2,13 +2,13 @@ package server
 
 import (
 	"fmt"
-	"io/ioutil"
+	"os"
 
 	"github.com/BurntSushi/toml"
 )
 
 func readLegacyConfig(path string) (*Config, error) {
-	data, err := ioutil.ReadFile(path)
+	data, err := os.ReadFile(path)
 	tomlData := string(data)
 
 	if err != nil {

internal/cli/server/helper.go

@@ -2,75 +2,36 @@ package server
 
 import (
 	"fmt"
-	"math/rand"
-	"net"
 	"os"
-	"sync/atomic"
-	"time"
-)
-
-var maxPortCheck int32 = 100
-
-// findAvailablePort returns the next available port starting from `from`
-func findAvailablePort(from int32, count int32) (int32, error) {
-	if count == maxPortCheck {
-		return 0, fmt.Errorf("no available port found")
-	}
 
-	port := atomic.AddInt32(&from, 1)
-	addr := fmt.Sprintf("localhost:%d", port)
-
-	count++
-
-	lis, err := net.Listen("tcp", addr)
-	if err == nil {
-		lis.Close()
-		return port, nil
-	} else {
-		return findAvailablePort(from, count)
-	}
-}
+	"github.com/ethereum/go-ethereum/common/network"
+)
 
 func CreateMockServer(config *Config) (*Server, error) {
 	if config == nil {
 		config = DefaultConfig()
 	}
 
-	// find available port for grpc server
-	rand.Seed(time.Now().UnixNano())
-
-	var (
-		from int32 = 60000 // the min port to start checking from
-		to   int32 = 61000 // the max port to start checking from
-	)
-
-	//nolint: gosec
-	port, err := findAvailablePort(rand.Int31n(to-from+1)+from, 0)
+	// get grpc port and listener
+	grpcPort, gRPCListener, err := network.FindAvailablePort()
 	if err != nil {
 		return nil, err
 	}
 
-	// grpc port
-	config.GRPC.Addr = fmt.Sprintf(":%d", port)
+	// The test uses grpc port from config so setting it here.
+	config.GRPC.Addr = fmt.Sprintf(":%d", grpcPort)
 
 	// datadir
-	datadir, _ := os.MkdirTemp("/tmp", "bor-cli-test")
-	config.DataDir = datadir
-
-	// find available port for http server
-	from = 8545
-	to = 9545
-
-	//nolint: gosec
-	port, err = findAvailablePort(rand.Int31n(to-from+1)+from, 0)
+	datadir, err := os.MkdirTemp("", "bor-cli-test")
 	if err != nil {
 		return nil, err
 	}
 
-	config.JsonRPC.Http.Port = uint64(port)
+	config.DataDir = datadir
+	config.JsonRPC.Http.Port = 0 // It will choose a free/available port
 
 	// start the server
-	return NewServer(config)
+	return NewServer(config, WithGRPCListener(gRPCListener))
 }
 
 func CloseMockServer(server *Server) {