e2e: Migrate duplicate node id test from kurtosis

.github/workflows/test.e2e.persistent.yml

@@ -28,7 +28,7 @@ jobs:
         run: ./scripts/build.sh -r
       - name: Run e2e tests with persistent network
         shell: bash
-        run: ./scripts/tests.e2e.persistent.sh ./build/avalanchego
+        run: E2E_SERIAL=1 ./scripts/tests.e2e.persistent.sh ./build/avalanchego
       - name: Upload testnet network dir
         uses: actions/upload-artifact@v3
         with:

.github/workflows/test.e2e.yml

@@ -28,7 +28,7 @@ jobs:
         run: ./scripts/build.sh -r
       - name: Run e2e tests
         shell: bash
-        run: ./scripts/tests.e2e.sh ./build/avalanchego
+        run: E2E_SERIAL=1 ./scripts/tests.e2e.sh ./build/avalanchego
       - name: Upload testnet network dir
         uses: actions/upload-artifact@v3
         with:

scripts/tests.e2e.sh

@@ -5,6 +5,7 @@ set -euo pipefail
 # e.g.,
 # ./scripts/build.sh
 # ./scripts/tests.e2e.sh ./build/avalanchego
+# E2E_SERIAL=1 ./scripts/tests.e2e.sh ./build/avalanchego
 if ! [[ "$0" =~ scripts/tests.e2e.sh ]]; then
   echo "must be run from repository root"
   exit 255
@@ -43,10 +44,28 @@ else
 fi
 
 #################################
-# - Execute in parallel (-p) with the ginkgo cli to minimize execution time.
-#   The test binary by itself isn't capable of running specs in parallel.
+# Determine ginkgo args
+GINKGO_ARGS=""
+if [[ -n "${E2E_SERIAL:-}" ]]; then
+  # Specs will be executed serially. This supports running e2e tests in CI
+  # where parallel execution of tests that start new nodes beyond the
+  # initial set of validators could overload the free tier CI workers.
+  # Forcing serial execution in this test script instead of marking
+  # resource-hungry tests as serial supports executing the test suite faster
+  # on powerful development workstations.
+  echo "tests will be executed serially to minimize resource requirements"
+else
+  # Enable parallel execution of specs defined in the test binary by
+  # default. This requires invoking the binary via the ginkgo cli
+  # since the test binary isn't capable of executing specs in
+  # parallel.
+  echo "tests will be executed in parallel"
+  GINKGO_ARGS="-p"
+fi
+
+#################################
 # - Execute in random order to identify unwanted dependency
-ginkgo -p -v --randomize-all ./tests/e2e/e2e.test -- ${E2E_ARGS} \
+ginkgo ${GINKGO_ARGS} -v --randomize-all ./tests/e2e/e2e.test -- ${E2E_ARGS} \
 && EXIT_CODE=$? || EXIT_CODE=$?
 
 if [[ ${EXIT_CODE} -gt 0 ]]; then

tests/e2e/e2e.go

@@ -36,6 +36,10 @@ const (
 	// Enough for test/custom networks.
 	DefaultConfirmTxTimeout = 20 * time.Second
 
+	// This interval should represent the upper bound of the time
+	// required to start a new node on a local test network.
+	DefaultNodeStartTimeout = 20 * time.Second
+
 	// A long default timeout used to timeout failed operations but
 	// unlikely to induce flaking due to unexpected resource
 	// contention.
@@ -168,3 +172,29 @@ func Eventually(condition func() bool, waitFor time.Duration, tick time.Duration
 		}
 	}
 }
+
+// Add an ephemeral node that is only intended to be used by a single test. Its ID and
+// URI are not intended to be returned from the Network instance to minimize
+// accessibility from other tests.
+func AddEphemeralNode(network testnet.Network, flags testnet.FlagsMap) testnet.Node {
+	require := require.New(ginkgo.GinkgoT())
+
+	node, err := network.AddEphemeralNode(ginkgo.GinkgoWriter, flags)
+	require.NoError(err)
+
+	// Ensure node is stopped on teardown. It's configuration is not removed to enable
+	// collection in CI to aid in troubleshooting failures.
+	ginkgo.DeferCleanup(func() {
+		tests.Outf("Shutting down ephemeral node %s\n", node.GetID())
+		require.NoError(node.Stop())
+	})
+
+	return node
+}
+
+// Wait for the given node to report healthy.
+func WaitForHealthy(node testnet.Node) {
+	ctx, cancel := context.WithTimeout(context.Background(), DefaultTimeout)
+	defer cancel()
+	require.NoError(ginkgo.GinkgoT(), testnet.WaitForHealthy(ctx, node))
+}

tests/e2e/e2e_test.go

@@ -26,6 +26,7 @@ import (
 	// ensure test packages are scanned by ginkgo
 	_ "github.com/ava-labs/avalanchego/tests/e2e/banff"
 	_ "github.com/ava-labs/avalanchego/tests/e2e/c"
+	_ "github.com/ava-labs/avalanchego/tests/e2e/faultinjection"
 	_ "github.com/ava-labs/avalanchego/tests/e2e/p"
 	_ "github.com/ava-labs/avalanchego/tests/e2e/static-handlers"
 	_ "github.com/ava-labs/avalanchego/tests/e2e/x"

tests/e2e/faultinjection/duplicate_node_id.go

@@ -0,0 +1,94 @@
+// Copyright (C) 2019-2023, Ava Labs, Inc. All rights reserved.
+// See the file LICENSE for licensing terms.
+
+package faultinjection
+
+import (
+	"context"
+	"fmt"
+
+	ginkgo "github.com/onsi/ginkgo/v2"
+
+	"github.com/stretchr/testify/require"
+
+	"github.com/ava-labs/avalanchego/api/info"
+	"github.com/ava-labs/avalanchego/config"
+	"github.com/ava-labs/avalanchego/ids"
+	"github.com/ava-labs/avalanchego/tests/e2e"
+	"github.com/ava-labs/avalanchego/tests/fixture/testnet"
+	"github.com/ava-labs/avalanchego/utils/set"
+)
+
+var _ = ginkgo.Describe("Duplicate node handling", func() {
+	require := require.New(ginkgo.GinkgoT())
+
+	ginkgo.It("should ensure that a given Node ID (i.e. staking keypair) can be used at most once on a network", func() {
+		network := e2e.Env.GetNetwork()
+		nodes := network.GetNodes()
+
+		ginkgo.By("creating new node")
+		node1 := e2e.AddEphemeralNode(network, testnet.FlagsMap{})
+		e2e.WaitForHealthy(node1)
+
+		ginkgo.By("checking that the new node is connected to its peers")
+		checkConnectedPeers(nodes, node1)
+
+		ginkgo.By("creating a second new node with the same staking keypair as the first new node")
+		node1Flags := node1.GetConfig().Flags
+		node2Flags := testnet.FlagsMap{
+			config.StakingTLSKeyContentKey: node1Flags[config.StakingTLSKeyContentKey],
+			config.StakingCertContentKey:   node1Flags[config.StakingCertContentKey],
+			// Construct a unique data dir to ensure the two nodes' data will be stored
+			// separately. Usually the dir name is the node ID but in this one case the nodes have
+			// the same node ID.
+			config.DataDirKey: fmt.Sprintf("%s-second", node1Flags[config.DataDirKey]),
+		}
+		node2 := e2e.AddEphemeralNode(network, node2Flags)
+
+		ginkgo.By("checking that the second new node fails to become healthy before timeout")
+		err := testnet.WaitForHealthy(e2e.DefaultContext(), node2)
+		require.ErrorIs(err, context.DeadlineExceeded)
+
+		ginkgo.By("stopping the first new node")
+		require.NoError(node1.Stop())
+
+		ginkgo.By("checking that the second new node becomes healthy within timeout")
+		e2e.WaitForHealthy(node2)
+
+		ginkgo.By("checking that the second new node is connected to its peers")
+		checkConnectedPeers(nodes, node2)
+	})
+})
+
+// Check that a new node is connected to existing nodes and vice versa
+func checkConnectedPeers(existingNodes []testnet.Node, newNode testnet.Node) {
+	require := require.New(ginkgo.GinkgoT())
+
+	// Collect the node ids of the new node's peers
+	infoClient := info.NewClient(newNode.GetProcessContext().URI)
+	peers, err := infoClient.Peers(context.Background())
+	require.NoError(err)
+	peerIDs := set.NewSet[ids.NodeID](len(existingNodes))
+	for _, peer := range peers {
+		peerIDs.Add(peer.ID)
+	}
+
+	newNodeID := newNode.GetID()
+	for _, existingNode := range existingNodes {
+		// Check that the existing node is a peer of the new node
+		require.True(peerIDs.Contains(existingNode.GetID()))
+
+		// Check that the new node is a peer
+		infoClient := info.NewClient(existingNode.GetProcessContext().URI)
+		peers, err := infoClient.Peers(context.Background())
+		require.NoError(err)
+		isPeer := false
+		for _, peer := range peers {
+			if peer.ID == newNodeID {
+				isPeer = true
+				break
+			}
+		}
+		require.True(isPeer)
+	}
+}

tests/fixture/testnet/common.go

@@ -0,0 +1,42 @@
+// Copyright (C) 2019-2023, Ava Labs, Inc. All rights reserved.
+// See the file LICENSE for licensing terms.
+
+package testnet
+
+import (
+	"context"
+	"errors"
+	"fmt"
+	"time"
+)
+
+const (
+	DefaultNodeTickerInterval = 50 * time.Millisecond
+)
+
+var ErrNotRunning = errors.New("not running")
+
+// WaitForHealthy blocks until Node.IsHealthy returns true or an error (including context timeout) is observed.
+func WaitForHealthy(ctx context.Context, node Node) error {
+	if _, ok := ctx.Deadline(); !ok {
+		return fmt.Errorf("unable to wait for health for node %q with a context without a deadline", node.GetID())
+	}
+	ticker := time.NewTicker(DefaultNodeTickerInterval)
+	defer ticker.Stop()
+
+	for {
+		healthy, err := node.IsHealthy(ctx)
+		if err != nil && !errors.Is(err, ErrNotRunning) {
+			return fmt.Errorf("failed to wait for health of node %q: %w", node.GetID(), err)
+		}
+		if healthy {
+			return nil
+		}
+
+		select {
+		case <-ctx.Done():
+			return fmt.Errorf("failed to wait for health of node %q before timeout: %w", node.GetID(), ctx.Err())
+		case <-ticker.C:
+		}
+	}
+}

tests/fixture/testnet/interfaces.go

@@ -4,6 +4,9 @@
 package testnet
 
 import (
+	"context"
+	"io"
+
 	"github.com/ava-labs/avalanchego/ids"
 	"github.com/ava-labs/avalanchego/node"
 )
@@ -12,11 +15,14 @@ import (
 type Network interface {
 	GetConfig() NetworkConfig
 	GetNodes() []Node
+	AddEphemeralNode(w io.Writer, flags FlagsMap) (Node, error)
 }
 
 // Defines node capabilities supportable regardless of how a network is orchestrated.
 type Node interface {
 	GetID() ids.NodeID
 	GetConfig() NodeConfig
 	GetProcessContext() node.NodeProcessContext
+	IsHealthy(ctx context.Context) (bool, error)
+	Stop() error
 }

tests/fixture/testnet/local/README.md

@@ -152,7 +152,10 @@ HOME
             │       └── config.json                      // C-Chain config for all nodes
             ├── defaults.json                            // Default flags and configuration for network
             ├── genesis.json                             // Genesis for all nodes
-            └── network.env                              // Sets network dir env to simplify use of network
+            ├── network.env                              // Sets network dir env to simplify use of network
+            └── ephemeral                                // Parent directory for ephemeral nodes (e.g. created by tests)
+                └─ NodeID-FdxnAvr4jK9XXAwsYZPgWAHW2QnwSZ // Data dir for an ephemeral node
+                   └── ...
 
 ```
 

tests/fixture/testnet/local/config.go

@@ -20,7 +20,6 @@ const (
 	DefaultNetworkStartTimeout = 2 * time.Minute
 	DefaultNodeInitTimeout     = 10 * time.Second
 	DefaultNodeStopTimeout     = 5 * time.Second
-	DefaultNodeTickerInterval  = 50 * time.Millisecond
 )
 
 // A set of flags appropriate for local testing.