Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

core: randomize p2p ports; consolidate GetFreePort implementations #10884

Merged
merged 1 commit into from
Oct 9, 2023
Merged

core: randomize p2p ports; consolidate GetFreePort implementations #10884

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
24 changes: 12 additions & 12 deletions core/internal/features/features_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down Expand Up @@ -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,
Expand All @@ -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)
Expand Down Expand Up @@ -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)

Expand All @@ -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)
Expand Down Expand Up @@ -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)

Expand All @@ -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))
Expand Down
13 changes: 5 additions & 8 deletions core/internal/features/ocr2/features_ocr2_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down Expand Up @@ -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 (
Expand Down Expand Up @@ -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 (
Expand Down
17 changes: 16 additions & 1 deletion core/internal/testutils/testutils.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ import (
"math"
"math/big"
mrand "math/rand"
"net"
"net/http"
"net/http/httptest"
"net/url"
Expand All @@ -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
Expand Down Expand Up @@ -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.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

FYI I asked Research about this here: https://chainlink-core.slack.com/archives/C042C3WPDNH/p1691442588046129

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)
}
22 changes: 5 additions & 17 deletions core/services/ocr2/plugins/functions/integration_tests/v0/internal/testutils.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,6 @@ import (
"io"
"math/big"
"math/rand"
"net"
"net/http"
"net/http/httptest"
"net/url"
Expand All @@ -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"
Expand Down Expand Up @@ -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)

Expand All @@ -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)
Expand All @@ -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,
Expand Down
22 changes: 5 additions & 17 deletions core/services/ocr2/plugins/functions/integration_tests/v1/internal/testutils.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,6 @@ import (
"io"
"math/big"
"math/rand"
"net"
"net/http"
"net/http/httptest"
"net/url"
Expand All @@ -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"
Expand Down Expand Up @@ -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)

Expand All @@ -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)
Expand All @@ -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,
Expand Down
22 changes: 5 additions & 17 deletions core/services/ocr2/plugins/ocr2vrf/internal/ocr2vrf_integration_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,6 @@ import (
"errors"
"fmt"
"math/big"
"net"
"testing"
"time"

Expand All @@ -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"
Expand Down Expand Up @@ -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

Expand All @@ -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()
Expand Down Expand Up @@ -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 }