Support non-validator nodes in e2e tests (#1378)

* Support non-validator nodes in e2e tests * Comments fix * Comments fix * Fix
0xPolygon · Apr 12, 2023 · de2bd8c · de2bd8c
1 parent ccbe2b9
commit de2bd8c
Show file tree

Hide file tree

Showing 7 changed files with 48 additions and 58 deletions.
diff --git a/command/genesis/genesis.go b/command/genesis/genesis.go
@@ -156,13 +156,6 @@ func setFlags(cmd *cobra.Command) {
 			"the manifest file path, which contains genesis metadata",
 		)
 
-		cmd.Flags().IntVar(
-			&params.validatorSetSize,
-			validatorSetSizeFlag,
-			defaultValidatorSetSize,
-			"the total number of validators",
-		)
-
 		cmd.Flags().Uint64Var(
 			&params.sprintSize,
 			sprintSizeFlag,

diff --git a/command/genesis/params.go b/command/genesis/params.go
@@ -79,7 +79,6 @@ type genesisParams struct {
 
 	// PolyBFT
 	manifestPath            string
-	validatorSetSize        int
 	sprintSize              uint64
 	blockTime               time.Duration
 	bridgeJSONRPCAddr       string

diff --git a/command/genesis/polybft_params.go b/command/genesis/polybft_params.go
@@ -24,7 +24,6 @@ import (
 
 const (
 	manifestPathFlag       = "manifest"
-	validatorSetSizeFlag   = "validator-set-size"
 	sprintSizeFlag         = "sprint-size"
 	blockTimeFlag          = "block-time"
 	bridgeFlag             = "bridge-json-rpc"

diff --git a/e2e-polybft/e2e/consensus_test.go b/e2e-polybft/e2e/consensus_test.go
@@ -4,6 +4,7 @@ import (
 	"fmt"
 	"math/big"
 	"path"
+	"strconv"
 	"testing"
 	"time"
 
@@ -22,19 +23,15 @@ import (
 func TestE2E_Consensus_Basic_WithNonValidators(t *testing.T) {
 	const epochSize = 4
 
-	//nolint:godox
-	// TODO: Enable non-validators when proper support gets implemented
-	// (https://polygon.atlassian.net/browse/EVM-547)
-	// framework.WithNonValidators(2),
-	cluster := framework.NewTestCluster(t, 7,
-		framework.WithEpochSize(epochSize))
+	cluster := framework.NewTestCluster(t, 5,
+		framework.WithEpochSize(epochSize), framework.WithNonValidators(2))
 	defer cluster.Stop()
 
 	t.Run("consensus protocol", func(t *testing.T) {
 		require.NoError(t, cluster.WaitForBlock(2*epochSize+1, 1*time.Minute))
 	})
 
-	t.Run("sync protool drop single validator node", func(t *testing.T) {
+	t.Run("sync protocol, drop single validator node", func(t *testing.T) {
 		// query the current block number, as it is a starting point for the test
 		currentBlockNum, err := cluster.Servers[0].JSONRPC().Eth().BlockNumber()
 		require.NoError(t, err)
@@ -52,32 +49,25 @@ func TestE2E_Consensus_Basic_WithNonValidators(t *testing.T) {
 		// wait 2 more epochs to elapse and make sure that stopped node managed to catch up
 		require.NoError(t, cluster.WaitForBlock(currentBlockNum+4*epochSize, 2*time.Minute))
 	})
-}
 
-func TestE2E_Consensus_Sync_WithNonValidators(t *testing.T) {
-	t.Skip("Enable once we have proper support for non-validators.")
-
-	// one non-validator node from the ensemble gets disconnected and connected again.
-	// It should be able to pick up from the synchronization protocol again.
-	cluster := framework.NewTestCluster(t, 7,
-		framework.WithNonValidators(2), framework.WithValidatorSnapshot(5))
-	defer cluster.Stop()
-
-	// wait for the start
-	require.NoError(t, cluster.WaitForBlock(20, 1*time.Minute))
+	t.Run("sync protocol, drop single non-validator node", func(t *testing.T) {
+		// query the current block number, as it is a starting point for the test
+		currentBlockNum, err := cluster.Servers[0].JSONRPC().Eth().BlockNumber()
+		require.NoError(t, err)
 
-	// stop one non-validator node
-	node := cluster.Servers[6]
-	node.Stop()
+		// stop one non-validator node
+		node := cluster.Servers[6]
+		node.Stop()
 
-	// wait for at least 15 more blocks before starting again
-	require.NoError(t, cluster.WaitForBlock(35, 2*time.Minute))
+		// wait for 2 epochs to elapse, so that rest of the network progresses
+		require.NoError(t, cluster.WaitForBlock(currentBlockNum+2*epochSize, 2*time.Minute))
 
-	// start the node again
-	node.Start()
+		// start the node again
+		node.Start()
 
-	// wait for block 55
-	require.NoError(t, cluster.WaitForBlock(55, 2*time.Minute))
+		// wait 2 more epochs to elapse and make sure that stopped node managed to catch up
+		require.NoError(t, cluster.WaitForBlock(currentBlockNum+4*epochSize, 2*time.Minute))
+	})
 }
 
 func TestE2E_Consensus_BulkDrop(t *testing.T) {
@@ -183,8 +173,8 @@ func TestE2E_Consensus_RegisterValidator(t *testing.T) {
 	require.NoError(t, owner.WhitelistValidator(secondValidatorAddr.String(), ownerSecrets))
 
 	// start the first and the second validator
-	cluster.InitTestServer(t, validatorSize+1, true, false)
-	cluster.InitTestServer(t, validatorSize+2, true, false)
+	cluster.InitTestServer(t, cluster.Config.ValidatorPrefix+strconv.Itoa(validatorSize+1), true, false)
+	cluster.InitTestServer(t, cluster.Config.ValidatorPrefix+strconv.Itoa(validatorSize+2), true, false)
 
 	ownerAcc, err := sidechain.GetAccountFromDir(path.Join(cluster.Config.TmpDir, ownerSecrets))
 	require.NoError(t, err)

diff --git a/e2e-polybft/e2e/migration_test.go b/e2e-polybft/e2e/migration_test.go
@@ -204,6 +204,6 @@ func TestE2E_Migration(t *testing.T) {
 	_, err = cluster.InitSecrets("test-chain-8", 1)
 	require.NoError(t, err)
 
-	cluster.InitTestServer(t, 8, false, false)
+	cluster.InitTestServer(t, "test-chain-8", false, false)
 	require.NoError(t, cluster.WaitForBlock(33, time.Minute))
 }
diff --git a/e2e-polybft/e2e/network_test.go b/e2e-polybft/e2e/network_test.go
@@ -20,8 +20,7 @@ func TestE2E_NetworkDiscoveryProtocol(t *testing.T) {
 	)
 
 	// create cluster
-	cluster := framework.NewTestCluster(t, 10,
-		framework.WithValidatorSnapshot(validatorCount),
+	cluster := framework.NewTestCluster(t, validatorCount,
 		framework.WithNonValidators(nonValidatorCount),
 		framework.WithBootnodeCount(1))
 	defer cluster.Stop()

diff --git a/e2e-polybft/framework/test-cluster.go b/e2e-polybft/framework/test-cluster.go
@@ -45,6 +45,9 @@ const (
 
 	// prefix for validator directory
 	defaultValidatorPrefix = "test-chain-"
+
+	// prefix for non validators directory
+	nonValidatorPrefix = "test-non-validator-"
 )
 
 var startTime int64
@@ -338,14 +341,28 @@ func NewTestCluster(t *testing.T, validatorsCount int, opts ...ClusterOption) *T
 		once:        sync.Once{},
 	}
 
+	// in case no validators are specified in opts, all nodes will be validators
+	if cluster.Config.ValidatorSetSize == 0 {
+		cluster.Config.ValidatorSetSize = uint64(validatorsCount)
+	}
+
 	{
-		// run init accounts
-		addresses, err := cluster.InitSecrets(cluster.Config.ValidatorPrefix, validatorsCount)
+		// run init accounts for validators
+		addresses, err := cluster.InitSecrets(cluster.Config.ValidatorPrefix, int(cluster.Config.ValidatorSetSize))
 		require.NoError(t, err)
 
 		if cluster.Config.SecretsCallback != nil {
 			cluster.Config.SecretsCallback(addresses, cluster.Config)
 		}
+
+		if config.NonValidatorCount > 0 {
+			// run init accounts for non-validators
+			_, err = cluster.InitSecrets(nonValidatorPrefix, config.NonValidatorCount)
+			require.NoError(t, err)
+
+			// we don't call secrets callback on non-validators,
+			// since we have nothing to premine nor stake for non validators
+		}
 	}
 
 	manifestPath := path.Join(config.TmpDir, "manifest.json")
@@ -374,11 +391,6 @@ func NewTestCluster(t *testing.T, validatorsCount int, opts ...ClusterOption) *T
 		require.NoError(t, err)
 	}
 
-	// in case no validators are specified in opts, all nodes will be validators
-	if cluster.Config.ValidatorSetSize == 0 {
-		cluster.Config.ValidatorSetSize = uint64(validatorsCount)
-	}
-
 	if cluster.Config.HasBridge {
 		err := cluster.Bridge.deployRootchainContracts(manifestPath)
 		require.NoError(t, err)
@@ -432,10 +444,6 @@ func NewTestCluster(t *testing.T, validatorsCount int, opts ...ClusterOption) *T
 			}
 		}
 
-		if cluster.Config.ValidatorSetSize > 0 {
-			args = append(args, "--validator-set-size", fmt.Sprint(cluster.Config.ValidatorSetSize))
-		}
-
 		if len(cluster.Config.ContractDeployerAllowListAdmin) != 0 {
 			args = append(args, "--contract-deployer-allow-list-admin",
 				strings.Join(sliceAddressToSliceString(cluster.Config.ContractDeployerAllowListAdmin), ","))
@@ -462,23 +470,25 @@ func NewTestCluster(t *testing.T, validatorsCount int, opts ...ClusterOption) *T
 	}
 
 	for i := 1; i <= int(cluster.Config.ValidatorSetSize); i++ {
-		cluster.InitTestServer(t, i, true, cluster.Config.HasBridge && i == 1 /* relayer */)
+		dir := cluster.Config.ValidatorPrefix + strconv.Itoa(i)
+		cluster.InitTestServer(t, dir, true, cluster.Config.HasBridge && i == 1 /* relayer */)
 	}
 
 	for i := 1; i <= cluster.Config.NonValidatorCount; i++ {
-		offsetIndex := i + int(cluster.Config.ValidatorSetSize)
-		cluster.InitTestServer(t, offsetIndex, false, false /* relayer */)
+		dir := nonValidatorPrefix + strconv.Itoa(i)
+		cluster.InitTestServer(t, dir, false, false /* relayer */)
 	}
 
 	return cluster
 }
 
-func (c *TestCluster) InitTestServer(t *testing.T, i int, isValidator bool, relayer bool) {
+func (c *TestCluster) InitTestServer(t *testing.T,
+	dataDir string, isValidator bool, relayer bool) {
 	t.Helper()
 
 	logLevel := os.Getenv(envLogLevel)
 
-	dataDir := c.Config.Dir(c.Config.ValidatorPrefix + strconv.Itoa(i))
+	dataDir = c.Config.Dir(dataDir)
 	if c.Config.InitialTrieDB != "" {
 		err := CopyDir(c.Config.InitialTrieDB, filepath.Join(dataDir, "trie"))
 		if err != nil {