core: randomize p2p ports; consolidate GetFreePort implementations (#…

…10884)

core/internal/features/features_test.go

@@ -670,12 +670,12 @@ func setupOCRContracts(t *testing.T) (*bind.TransactOpts, *backends.SimulatedBac
 	return owner, b, ocrContractAddress, ocrContract, flagsContract, flagsContractAddress
 }
 
-func setupNode(t *testing.T, owner *bind.TransactOpts, portV1, portV2 int, dbName string,
+func setupNode(t *testing.T, owner *bind.TransactOpts, portV1, portV2 uint16, dbName string,
 	b *backends.SimulatedBackend, ns ocrnetworking.NetworkingStack, overrides func(c *chainlink.Config, s *chainlink.Secrets),
 ) (*cltest.TestApplication, string, common.Address, ocrkey.KeyV2) {
 	p2pKey, err := p2pkey.NewV2()
 	require.NoError(t, err)
-	config, _ := heavyweight.FullTestDBV2(t, fmt.Sprintf("%s%d", dbName, portV1), func(c *chainlink.Config, s *chainlink.Secrets) {
+	config, _ := heavyweight.FullTestDBV2(t, dbName, func(c *chainlink.Config, s *chainlink.Secrets) {
 		c.Insecure.OCRDevelopmentMode = ptr(true) // Disables ocr spec validation so we can have fast polling for the test.
 
 		c.OCR.Enabled = ptr(true)
@@ -743,7 +743,7 @@ func setupForwarderEnabledNode(
 	t *testing.T,
 	owner *bind.TransactOpts,
 	portV1,
-	portV2 int,
+	portV2 uint16,
 	dbName string,
 	b *backends.SimulatedBackend,
 	ns ocrnetworking.NetworkingStack,
@@ -757,7 +757,7 @@ func setupForwarderEnabledNode(
 ) {
 	p2pKey, err := p2pkey.NewV2()
 	require.NoError(t, err)
-	config, _ := heavyweight.FullTestDBV2(t, fmt.Sprintf("%s%d", dbName, portV1), func(c *chainlink.Config, s *chainlink.Secrets) {
+	config, _ := heavyweight.FullTestDBV2(t, dbName, func(c *chainlink.Config, s *chainlink.Secrets) {
 		c.Insecure.OCRDevelopmentMode = ptr(true) // Disables ocr spec validation so we can have fast polling for the test.
 
 		c.OCR.Enabled = ptr(true)
@@ -856,8 +856,8 @@ func TestIntegration_OCR(t *testing.T) {
 	for _, tt := range tests {
 		test := tt
 		t.Run(test.name, func(t *testing.T) {
-			bootstrapNodePortV1 := test.portStart
-			bootstrapNodePortV2 := test.portStart + numOracles + 1
+			bootstrapNodePortV1 := testutils.GetFreePort(t)
+			bootstrapNodePortV2 := testutils.GetFreePort(t)
 			g := gomega.NewWithT(t)
 			owner, b, ocrContractAddress, ocrContract, flagsContract, flagsContractAddress := setupOCRContracts(t)
 
@@ -871,8 +871,8 @@ func TestIntegration_OCR(t *testing.T) {
 				apps         []*cltest.TestApplication
 			)
 			for i := 0; i < numOracles; i++ {
-				portV1 := bootstrapNodePortV1 + i + 1
-				portV2 := bootstrapNodePortV2 + i + 1
+				portV1 := testutils.GetFreePort(t)
+				portV2 := testutils.GetFreePort(t)
 				app, peerID, transmitter, key := setupNode(t, owner, portV1, portV2, fmt.Sprintf("o%d_%d", i, test.id), b, test.ns, func(c *chainlink.Config, s *chainlink.Secrets) {
 					c.EVM[0].FlagsContractAddress = ptr(ethkey.EIP55AddressFromAddress(flagsContractAddress))
 					c.EVM[0].GasEstimator.EIP1559DynamicFees = ptr(test.eip1559)
@@ -1080,8 +1080,8 @@ func TestIntegration_OCR_ForwarderFlow(t *testing.T) {
 	t.Parallel()
 	numOracles := 4
 	t.Run("ocr_forwarder_flow", func(t *testing.T) {
-		bootstrapNodePortV1 := 20000
-		bootstrapNodePortV2 := 20000 + numOracles + 1
+		bootstrapNodePortV1 := testutils.GetFreePort(t)
+		bootstrapNodePortV2 := testutils.GetFreePort(t)
 		g := gomega.NewWithT(t)
 		owner, b, ocrContractAddress, ocrContract, flagsContract, flagsContractAddress := setupOCRContracts(t)
 
@@ -1097,8 +1097,8 @@ func TestIntegration_OCR_ForwarderFlow(t *testing.T) {
 			apps                []*cltest.TestApplication
 		)
 		for i := 0; i < numOracles; i++ {
-			portV1 := bootstrapNodePortV1 + i + 1
-			portV2 := bootstrapNodePortV2 + i + 1
+			portV1 := testutils.GetFreePort(t)
+			portV2 := testutils.GetFreePort(t)
 			app, peerID, transmitter, forwarder, key := setupForwarderEnabledNode(t, owner, portV1, portV2, fmt.Sprintf("o%d_%d", i, 1), b, ocrnetworking.NetworkingStackV2, func(c *chainlink.Config, s *chainlink.Secrets) {
 				c.Feature.LogPoller = ptr(true)
 				c.EVM[0].FlagsContractAddress = ptr(ethkey.EIP55AddressFromAddress(flagsContractAddress))

core/internal/features/ocr2/features_ocr2_test.go

@@ -23,11 +23,12 @@ import (
 	"github.com/ethereum/go-ethereum/core/types"
 	"github.com/ethereum/go-ethereum/eth/ethconfig"
 	"github.com/onsi/gomega"
-	"github.com/smartcontractkit/libocr/commontypes"
-	"github.com/smartcontractkit/libocr/gethwrappers2/ocr2aggregator"
 	"github.com/stretchr/testify/assert"
 	"github.com/stretchr/testify/require"
 
+	"github.com/smartcontractkit/libocr/commontypes"
+	"github.com/smartcontractkit/libocr/gethwrappers2/ocr2aggregator"
+
 	testoffchainaggregator2 "github.com/smartcontractkit/libocr/gethwrappers2/testocr2aggregator"
 	confighelper2 "github.com/smartcontractkit/libocr/offchainreporting2plus/confighelper"
 	ocrtypes2 "github.com/smartcontractkit/libocr/offchainreporting2plus/types"
@@ -191,9 +192,7 @@ func TestIntegration_OCR2(t *testing.T) {
 	owner, b, ocrContractAddress, ocrContract := setupOCR2Contracts(t)
 
 	lggr := logger.TestLogger(t)
-	// Note it's plausible these ports could be occupied on a CI machine.
-	// May need a port randomize + retry approach if we observe collisions.
-	bootstrapNodePort := uint16(29999)
+	bootstrapNodePort := testutils.GetFreePort(t)
 	bootstrapNode := setupNodeOCR2(t, owner, bootstrapNodePort, "bootstrap", false /* useForwarders */, b, nil)
 
 	var (
@@ -462,9 +461,7 @@ func TestIntegration_OCR2_ForwarderFlow(t *testing.T) {
 	owner, b, ocrContractAddress, ocrContract := setupOCR2Contracts(t)
 
 	lggr := logger.TestLogger(t)
-	// Note it's plausible these ports could be occupied on a CI machine.
-	// May need a port randomize + retry approach if we observe collisions.
-	bootstrapNodePort := uint16(29898)
+	bootstrapNodePort := testutils.GetFreePort(t)
 	bootstrapNode := setupNodeOCR2(t, owner, bootstrapNodePort, "bootstrap", true /* useForwarders */, b, nil)
 
 	var (

core/internal/testutils/testutils.go

@@ -10,6 +10,7 @@ import (
 	"math"
 	"math/big"
 	mrand "math/rand"
+	"net"
 	"net/http"
 	"net/http/httptest"
 	"net/url"
@@ -24,10 +25,11 @@ import (
 	"github.com/ethereum/go-ethereum/crypto"
 	"github.com/google/uuid"
 	"github.com/gorilla/websocket"
-	"github.com/smartcontractkit/sqlx"
 	"github.com/tidwall/gjson"
 	"go.uber.org/zap/zaptest/observer"
 
+	"github.com/smartcontractkit/sqlx"
+
 	"github.com/stretchr/testify/assert"
 	"github.com/stretchr/testify/require"
 	// NOTE: To avoid circular dependencies, this package MUST NOT import
@@ -450,3 +452,16 @@ func MustDecodeBase64(s string) (b []byte) {
 	}
 	return
 }
+
+// GetFreePort returns a free port.
+// NOTE: This approach is technically incorrect because the returned port
+// can still be taken by the time the caller attempts to bind to it.
+// Unfortunately, we can't specify zero port in P2P.V2.ListenAddresses at the moment.
+func GetFreePort(t *testing.T) uint16 {
+	addr, err := net.ResolveTCPAddr("tcp", "localhost:0")
+	require.NoError(t, err)
+	listener, err := net.ListenTCP("tcp", addr)
+	require.NoError(t, err)
+	require.NoError(t, listener.Close())
+	return uint16(listener.Addr().(*net.TCPAddr).Port)
+}

core/services/ocr2/plugins/functions/integration_tests/v0/internal/testutils.go

@@ -7,7 +7,6 @@ import (
 	"io"
 	"math/big"
 	"math/rand"
-	"net"
 	"net/http"
 	"net/http/httptest"
 	"net/url"
@@ -22,11 +21,12 @@ import (
 	"github.com/ethereum/go-ethereum/core/types"
 	"github.com/ethereum/go-ethereum/eth/ethconfig"
 	"github.com/onsi/gomega"
+	"github.com/stretchr/testify/assert"
+	"github.com/stretchr/testify/require"
+
 	"github.com/smartcontractkit/libocr/commontypes"
 	confighelper2 "github.com/smartcontractkit/libocr/offchainreporting2plus/confighelper"
 	ocrtypes2 "github.com/smartcontractkit/libocr/offchainreporting2plus/types"
-	"github.com/stretchr/testify/assert"
-	"github.com/stretchr/testify/require"
 
 	"github.com/smartcontractkit/chainlink/v2/core/assets"
 	"github.com/smartcontractkit/chainlink/v2/core/bridges"
@@ -469,7 +469,7 @@ func CreateFunctionsNodes(
 		require.Fail(t, "ocr2Keystores and thresholdKeyShares must have the same length")
 	}
 
-	bootstrapPort := getFreePort(t)
+	bootstrapPort := testutils.GetFreePort(t)
 	bootstrapNode = StartNewNode(t, owner, bootstrapPort, "bootstrap", b, uint32(maxGas), nil, nil, "")
 	AddBootstrapJob(t, bootstrapNode.App, oracleContractAddress)
 
@@ -487,7 +487,7 @@ func CreateFunctionsNodes(
 		} else {
 			ocr2Keystore = ocr2Keystores[i]
 		}
-		nodePort := getFreePort(t)
+		nodePort := testutils.GetFreePort(t)
 		oracleNode := StartNewNode(t, owner, nodePort, fmt.Sprintf("oracle%d", i), b, uint32(maxGas), []commontypes.BootstrapperLocator{
 			{PeerID: bootstrapNode.PeerID, Addrs: []string{fmt.Sprintf("127.0.0.1:%d", bootstrapPort)}},
 		}, ocr2Keystore, thresholdKeyShare)
@@ -503,18 +503,6 @@ func CreateFunctionsNodes(
 	return bootstrapNode, oracleNodes, oracleIdentites
 }
 
-// NOTE: This approach is technically incorrect because the returned port
-// can still be taken by the time the caller attempts to bind to it.
-// Unfortunately, we can't specify zero port in P2P.V2.ListenAddresses at the moment.
-func getFreePort(t *testing.T) uint16 {
-	addr, err := net.ResolveTCPAddr("tcp", "localhost:0")
-	require.NoError(t, err)
-	listener, err := net.ListenTCP("tcp", addr)
-	require.NoError(t, err)
-	require.NoError(t, listener.Close())
-	return uint16(listener.Addr().(*net.TCPAddr).Port)
-}
-
 func ClientTestRequests(
 	t *testing.T,
 	owner *bind.TransactOpts,

core/services/ocr2/plugins/functions/integration_tests/v1/internal/testutils.go

@@ -7,7 +7,6 @@ import (
 	"io"
 	"math/big"
 	"math/rand"
-	"net"
 	"net/http"
 	"net/http/httptest"
 	"net/url"
@@ -23,11 +22,12 @@ import (
 	"github.com/ethereum/go-ethereum/crypto"
 	"github.com/ethereum/go-ethereum/eth/ethconfig"
 	"github.com/onsi/gomega"
+	"github.com/stretchr/testify/assert"
+	"github.com/stretchr/testify/require"
+
 	"github.com/smartcontractkit/libocr/commontypes"
 	confighelper2 "github.com/smartcontractkit/libocr/offchainreporting2plus/confighelper"
 	ocrtypes2 "github.com/smartcontractkit/libocr/offchainreporting2plus/types"
-	"github.com/stretchr/testify/assert"
-	"github.com/stretchr/testify/require"
 
 	"github.com/smartcontractkit/chainlink/v2/core/assets"
 	"github.com/smartcontractkit/chainlink/v2/core/bridges"
@@ -549,7 +549,7 @@ func CreateFunctionsNodes(
 		require.Fail(t, "ocr2Keystores and thresholdKeyShares must have the same length")
 	}
 
-	bootstrapPort := getFreePort(t)
+	bootstrapPort := testutils.GetFreePort(t)
 	bootstrapNode = StartNewNode(t, owner, bootstrapPort, "bootstrap", b, uint32(maxGas), nil, nil, "")
 	AddBootstrapJob(t, bootstrapNode.App, routerAddress)
 
@@ -567,7 +567,7 @@ func CreateFunctionsNodes(
 		} else {
 			ocr2Keystore = ocr2Keystores[i]
 		}
-		nodePort := getFreePort(t)
+		nodePort := testutils.GetFreePort(t)
 		oracleNode := StartNewNode(t, owner, nodePort, fmt.Sprintf("oracle%d", i), b, uint32(maxGas), []commontypes.BootstrapperLocator{
 			{PeerID: bootstrapNode.PeerID, Addrs: []string{fmt.Sprintf("127.0.0.1:%d", bootstrapPort)}},
 		}, ocr2Keystore, thresholdKeyShare)
@@ -583,18 +583,6 @@ func CreateFunctionsNodes(
 	return bootstrapNode, oracleNodes, oracleIdentites
 }
 
-// NOTE: This approach is technically incorrect because the returned port
-// can still be taken by the time the caller attempts to bind to it.
-// Unfortunately, we can't specify zero port in P2P.V2.ListenAddresses at the moment.
-func getFreePort(t *testing.T) uint16 {
-	addr, err := net.ResolveTCPAddr("tcp", "localhost:0")
-	require.NoError(t, err)
-	listener, err := net.ListenTCP("tcp", addr)
-	require.NoError(t, err)
-	require.NoError(t, listener.Close())
-	return uint16(listener.Addr().(*net.TCPAddr).Port)
-}
-
 func ClientTestRequests(
 	t *testing.T,
 	owner *bind.TransactOpts,

core/services/ocr2/plugins/ocr2vrf/internal/ocr2vrf_integration_test.go

@@ -7,7 +7,6 @@ import (
 	"errors"
 	"fmt"
 	"math/big"
-	"net"
 	"testing"
 	"time"
 
@@ -20,16 +19,16 @@ import (
 	"github.com/ethereum/go-ethereum/crypto"
 	"github.com/ethereum/go-ethereum/eth/ethconfig"
 	"github.com/onsi/gomega"
+	"github.com/stretchr/testify/require"
+	"go.dedis.ch/kyber/v3"
+
 	"github.com/smartcontractkit/libocr/commontypes"
 	confighelper2 "github.com/smartcontractkit/libocr/offchainreporting2plus/confighelper"
 	ocrtypes2 "github.com/smartcontractkit/libocr/offchainreporting2plus/types"
 	"github.com/smartcontractkit/ocr2vrf/altbn_128"
 	ocr2dkg "github.com/smartcontractkit/ocr2vrf/dkg"
 	"github.com/smartcontractkit/ocr2vrf/ocr2vrf"
 	ocr2vrftypes "github.com/smartcontractkit/ocr2vrf/types"
-	"github.com/stretchr/testify/assert"
-	"github.com/stretchr/testify/require"
-	"go.dedis.ch/kyber/v3"
 
 	"github.com/smartcontractkit/chainlink/v2/core/assets"
 	"github.com/smartcontractkit/chainlink/v2/core/chains/evm/forwarders"
@@ -337,7 +336,7 @@ func runOCR2VRFTest(t *testing.T, useForwarders bool) {
 
 	t.Log("Creating bootstrap node")
 
-	bootstrapNodePort := getFreePort(t)
+	bootstrapNodePort := testutils.GetFreePort(t)
 	bootstrapNode := setupNodeOCR2(t, uni.owner, bootstrapNodePort, "bootstrap", uni.backend, false, nil)
 	numNodes := 5
 
@@ -362,7 +361,7 @@ func runOCR2VRFTest(t *testing.T, useForwarders bool) {
 				fmt.Sprintf("127.0.0.1:%d", bootstrapNodePort),
 			}},
 		}
-		node := setupNodeOCR2(t, uni.owner, getFreePort(t), fmt.Sprintf("ocr2vrforacle%d", i), uni.backend, useForwarders, bootstrappers)
+		node := setupNodeOCR2(t, uni.owner, testutils.GetFreePort(t), fmt.Sprintf("ocr2vrforacle%d", i), uni.backend, useForwarders, bootstrappers)
 		sendingKeys = append(sendingKeys, node.sendingKeys)
 
 		dkgSignKey, err := node.app.GetKeyStore().DKGSign().Create()
@@ -875,15 +874,4 @@ func randomKeyID(t *testing.T) (r [32]byte) {
 	return
 }
 
-func getFreePort(t *testing.T) uint16 {
-	addr, err := net.ResolveTCPAddr("tcp", "localhost:0")
-	require.NoError(t, err)
-
-	l, err := net.ListenTCP("tcp", addr)
-	require.NoError(t, err)
-	defer func() { assert.NoError(t, l.Close()) }()
-
-	return uint16(l.Addr().(*net.TCPAddr).Port)
-}
-
 func ptr[T any](v T) *T { return &v }