Rootchain CLI commands parallelization follow up (#1473)

* Small refactor of IsValidAddress

* Output successful results even in case of errors for rootchain fund and rootchain deploy commands

* Misspelling fix

* Remove leftover

* Filter out nil results

* Make error msg more verbose

* Fix timeout in register validator e2e test

command/rootchain/deploy/deploy.go

@@ -427,9 +427,10 @@ func deployContracts(outputter command.OutputFormatter, client *jsonrpc.Client,
 	allContracts = append(tokenContracts, allContracts...)
 
 	g, ctx := errgroup.WithContext(cmdCtx)
-	resultsCh := make(chan *deployContractResult, len(allContracts))
+	results := make([]*deployContractResult, len(allContracts))
 
-	for _, contract := range allContracts {
+	for i, contract := range allContracts {
+		i := i
 		contract := contract
 
 		g.Go(func() error {
@@ -444,14 +445,14 @@ func deployContracts(outputter command.OutputFormatter, client *jsonrpc.Client,
 
 				receipt, err := txRelayer.SendTransaction(txn, deployerKey)
 				if err != nil {
-					return err
+					return fmt.Errorf("failed sending %s contract deploy transaction: %w", contract.name, err)
 				}
 
 				if receipt == nil || receipt.Status != uint64(types.ReceiptSuccess) {
 					return fmt.Errorf("deployment of %s contract failed", contract.name)
 				}
 
-				resultsCh <- newDeployContractsResult(contract.name,
+				results[i] = newDeployContractsResult(contract.name,
 					types.Address(receipt.ContractAddress),
 					receipt.TransactionHash)
 
@@ -461,12 +462,20 @@ func deployContracts(outputter command.OutputFormatter, client *jsonrpc.Client,
 	}
 
 	if err := g.Wait(); err != nil {
+		_, _ = outputter.Write([]byte("[ROOTCHAIN - DEPLOY] Successfully deployed the following contracts\n"))
+
+		for _, result := range results {
+			if result != nil {
+				// In case an error happened, some of the indices may not be populated.
+				// Filter those out.
+				outputter.WriteCommandResult(result)
+			}
+		}
+
 		return nil, 0, err
 	}
 
-	close(resultsCh)
-
-	for result := range resultsCh {
+	for _, result := range results {
 		populatorFn, ok := metadataPopulatorMap[result.Name]
 		if !ok {
 			return nil, 0, fmt.Errorf("rootchain metadata populator not registered for contract '%s'", result.Name)

command/rootchain/fund/fund.go

@@ -107,7 +107,7 @@ func runCommand(cmd *cobra.Command, _ []string) {
 		rootTokenAddr = types.StringToAddress(params.nativeRootTokenAddr)
 	}
 
-	cmdResults := make(chan command.CommandResult, len(params.addresses))
+	results := make([]command.CommandResult, len(params.addresses))
 	g, ctx := errgroup.WithContext(cmd.Context())
 
 	for i := 0; i < len(params.addresses); i++ {
@@ -153,7 +153,7 @@ func runCommand(cmd *cobra.Command, _ []string) {
 					}
 				}
 
-				cmdResults <- &result{
+				results[i] = &result{
 					ValidatorAddr: validatorAddr,
 					TxHash:        types.Hash(receipt.TransactionHash),
 					IsMinted:      params.mintRootToken,
@@ -166,15 +166,17 @@ func runCommand(cmd *cobra.Command, _ []string) {
 
 	if err := g.Wait(); err != nil {
 		outputter.SetError(err)
+		_, _ = outputter.Write([]byte("[ROOTCHAIN FUND] Successfully funded following accounts\n"))
 
-		return
-	}
-
-	close(cmdResults)
+		for _, result := range results {
+			if result != nil {
+				// In case an error happened, some of the indices may not be populated.
+				// Filter those out.
+				outputter.SetCommandResult(result)
+			}
+		}
 
-	results := []command.CommandResult{}
-	for result := range cmdResults {
-		results = append(results, result)
+		return
 	}
 
 	outputter.SetCommandResult(command.Results(results))

e2e-polybft/e2e/consensus_test.go

@@ -272,8 +272,11 @@ func TestE2E_Consensus_RegisterValidator(t *testing.T) {
 	require.True(t, isFirstValidatorFound)
 	require.True(t, isSecondValidatorFound)
 
+	currentBlock, err := owner.JSONRPC().Eth().GetBlockByNumber(ethgo.Latest, false)
+	require.NoError(t, err)
+
 	// wait for couple of epochs to have some rewards accumulated
-	require.NoError(t, cluster.WaitForBlock(polybftConfig.EpochSize*7, time.Minute))
+	require.NoError(t, cluster.WaitForBlock(currentBlock.Number+(polybftConfig.EpochSize*2), time.Minute))
 
 	bigZero := big.NewInt(0)
 

types/types.go

@@ -140,16 +140,17 @@ func IsValidAddress(address string) error {
 		address = address[2:]
 	}
 
-	// check if the address has the correct length
-	if len(address) != 2*AddressLength {
-		return fmt.Errorf("address %s has invalid length", address)
-	}
-
 	// decode the address
-	if _, err := hex.DecodeString(address); err != nil {
+	decodedAddress, err := hex.DecodeString(address)
+	if err != nil {
 		return fmt.Errorf("address %s contains invalid characters", address)
 	}
 
+	// check if the address has the correct length
+	if len(decodedAddress) != AddressLength {
+		return fmt.Errorf("address %s has invalid length", address)
+	}
+
 	return nil
 }
 

types/types_test.go

@@ -6,6 +6,7 @@ import (
 	"testing"
 
 	"github.com/stretchr/testify/assert"
+	"github.com/stretchr/testify/require"
 )
 
 func TestEIP55(t *testing.T) {
@@ -86,3 +87,28 @@ func TestTransactionCopy(t *testing.T) {
 		t.Fatal("[ERROR] Copied transaction not equal base transaction")
 	}
 }
+
+func TestIsValidAddress(t *testing.T) {
+	t.Parallel()
+
+	cases := []struct {
+		address string
+		isValid bool
+	}{
+		{address: "0x123", isValid: false},
+		{address: "FooBar", isValid: false},
+		{address: "123FooBar", isValid: false},
+		{address: "0x1234567890987654321012345678909876543210", isValid: true},
+		{address: "0x0000000000000000000000000000000000000000", isValid: true},
+		{address: "0x1000000000000000000000000000000000000000", isValid: true},
+	}
+
+	for _, c := range cases {
+		err := IsValidAddress(c.address)
+		if c.isValid {
+			require.NoError(t, err)
+		} else {
+			require.Error(t, err)
+		}
+	}
+}