fix possible nil pointer, remove block range timeout, mutex for error…

… appending

integration-tests/actions/seth/actions.go

@@ -3,13 +3,9 @@ package actions_seth
 import (
 	"context"
 	"crypto/ecdsa"
-	"encoding/hex"
-	"encoding/json"
 	"fmt"
 	"math"
 	"math/big"
-	"os"
-	"path/filepath"
 	"strings"
 	"testing"
 	"time"
@@ -550,55 +546,6 @@ func TeardownRemoteSuite(
 				"Environment is left running so you can try manually!")
 	}
 
-	// This is a failsafe, we should never use ephemeral keys on live networks
-	if !client.Cfg.IsSimulatedNetwork() {
-		if err := ReturnFundFromEphemeralKeys(l, client); err != nil {
-			l.Error().Err(err).Str("Namespace", namespace).
-				Msg("Error attempting to return funds from ephemeral keys to root key")
-
-			pkStrings := []string{}
-			for _, pk := range client.PrivateKeys {
-				privateKeyBytes := pk.D.Bytes()
-				privateKeyBytes32 := make([]byte, 32)
-				copy(privateKeyBytes32[32-len(privateKeyBytes):], privateKeyBytes)
-				privateKeyHex := hex.EncodeToString(privateKeyBytes32)
-				pkStrings = append(pkStrings, "0x"+privateKeyHex)
-			}
-
-			privateKeyJson, jsonErr := json.Marshal(pkStrings)
-			if jsonErr != nil {
-				l.Error().
-					Err(jsonErr).
-					Msg("Error marshalling private keys to JSON. Funds are left in ephemeral keys")
-
-				return err
-			}
-
-			fileName := "ephemeral_addresses_private_keys.json"
-			writeErr := os.WriteFile(fileName, privateKeyJson, 0600)
-			if writeErr != nil {
-				l.Error().
-					Err(writeErr).
-					Msg("Error writing ephemeral addresses private keys to file. Funds are left in ephemeral keys")
-
-				return err
-			}
-			absolutePath, pathErr := filepath.Abs(fileName)
-			if pathErr != nil {
-				l.Error().
-					Err(pathErr).
-					Str("FileName", fileName).
-					Msg("Error getting absolute path of file with private keys. Try looking for it yourself.")
-
-				return err
-			}
-
-			l.Info().
-				Str("Filepath", absolutePath).
-				Msg("Private keys for ephemeral addresses are saved in the file. You can use them to return funds manually.")
-		}
-	}
-
 	return err
 }
 

integration-tests/actions/seth/keeper_helpers.go

@@ -11,6 +11,8 @@ import (
 
 	"github.com/ethereum/go-ethereum/common"
 	"github.com/ethereum/go-ethereum/core/types"
+	"github.com/google/uuid"
+	"github.com/pkg/errors"
 	"github.com/smartcontractkit/seth"
 	"github.com/stretchr/testify/require"
 
@@ -224,7 +226,7 @@ func RegisterUpkeepContracts(t *testing.T, client *seth.Client, linkToken contra
 		numberOfContracts, upkeepAddresses, checkData, isLogTrigger, isMercury)
 }
 
-// RegisterUpkeepContractsWithCheckData concurrently registers a set of upkeep contracts with the given check data. It requires at least 1 ephemeral key to be present in Seth config.
+// RegisterUpkeepContractsWithCheckData concurrently registers a set of upkeep contracts with the given check data. It requires at least 1 extra private key (epehemeral or static) to be present in Seth config.
 func RegisterUpkeepContractsWithCheckData(t *testing.T, client *seth.Client, linkToken contracts.LinkToken, linkFunds *big.Int, upkeepGasLimit uint32, registry contracts.KeeperRegistry, registrar contracts.KeeperRegistrar, numberOfContracts int, upkeepAddresses []string, checkData [][]byte, isLogTrigger bool, isMercury bool) []*big.Int {
 	l := logging.GetTestLogger(t)
 	registrationTxHashes := make([]common.Hash, 0)
@@ -253,14 +255,12 @@ func RegisterUpkeepContractsWithCheckData(t *testing.T, client *seth.Client, lin
 	var wgProcesses sync.WaitGroup
 	wgProcesses.Add(len(upkeepAddresses))
 
-	deplymentErrors := []error{}
-	deploymentCh := make(chan result, numberOfContracts)
-
 	// used mostly for logging/visibility and upkeep name creation
 	atomicCounter := atomic.Uint64{}
 
 	var registerUpkeepFn = func(channel chan result, keyNum int, config config) {
 		atomicCounter.Add(1)
+		uuid := uuid.New().String()
 
 		req, err := registrar.EncodeRegisterRequest(
 			fmt.Sprintf("upkeep_%d", atomicCounter.Load()),
@@ -277,34 +277,40 @@ func RegisterUpkeepContractsWithCheckData(t *testing.T, client *seth.Client, lin
 		)
 
 		if err != nil {
-			channel <- result{err: err}
+			channel <- result{err: errors.Wrapf(err, "[uuid: %s] Failed to encode register request for upkeep %s", uuid, config.address)}
 			return
 		}
 
 		balance, err := linkToken.BalanceOf(context.Background(), client.Addresses[keyNum].Hex())
 		if err != nil {
-			channel <- result{err: fmt.Errorf("Failed to get LINK balance of %s: %w", client.Addresses[keyNum].Hex(), err)}
+			channel <- result{err: errors.Wrapf(err, "[uuid: %s]Failed to get LINK balance of %s", uuid, client.Addresses[keyNum].Hex())}
 			return
 		}
 
 		// not stricly necessary, but helps us to avoid an errorless revert if there is not enough LINK
 		if balance.Cmp(linkFunds) < 0 {
-			channel <- result{err: fmt.Errorf("Not enough LINK balance for %s. Has: %s. Needs: %s", client.Addresses[keyNum].Hex(), balance.String(), linkFunds.String())}
+			channel <- result{err: fmt.Errorf("[uuid: %s] Not enough LINK balance for %s. Has: %s. Needs: %s", uuid, client.Addresses[keyNum].Hex(), balance.String(), linkFunds.String())}
 			return
 		}
 
 		tx, err := linkToken.TransferAndCallFromKey(registrar.Address(), linkFunds, req, keyNum)
-		channel <- result{tx: tx, err: err}
+		channel <- result{tx: tx, err: errors.Wrapf(err, "[uuid: %s] Failed to register upkeep %s", uuid, config.address)}
 	}
 
+	deploymentCh := make(chan result, numberOfContracts)
+	deplymentErrors := []error{}
+	errMut := sync.Mutex{}
+
 	// listen in the background until all registrations are done (it won't fail on the first error)
 	go func() {
 		defer l.Debug().Msg("Finished listening to results of registering upkeeps")
 		for r := range deploymentCh {
 			if r.err != nil {
+				errMut.Lock()
 				l.Error().Err(r.err).Msg("Failed to register upkeep")
 				deplymentErrors = append(deplymentErrors, r.err)
 				wgProcesses.Done()
+				errMut.Unlock()
 				continue
 			}
 
@@ -344,7 +350,7 @@ func RegisterUpkeepContractsWithCheckData(t *testing.T, client *seth.Client, lin
 
 	require.Equal(t, 0, len(deplymentErrors), "Failed to register some upkeeps")
 
-	// Fetch the upkeep IDs
+	// Fetch the upkeep IDs (this could be moved to registerUpkeepFn to speed it up a bit)
 	for _, txHash := range registrationTxHashes {
 		receipt, err := client.Client.TransactionReceipt(context.Background(), txHash)
 		require.NoError(t, err, "Registration tx should be completed")

integration-tests/actions/seth/refund.go

@@ -17,7 +17,6 @@ import (
 	"github.com/pkg/errors"
 	"github.com/rs/zerolog"
 	"github.com/smartcontractkit/seth"
-	"golang.org/x/sync/errgroup"
 
 	"github.com/smartcontractkit/chainlink-testing-framework/blockchain"
 
@@ -368,21 +367,3 @@ func sendAllFundsIfPossible(log zerolog.Logger, sethClient *seth.Client, fromPri
 
 	return nil
 }
-
-// ReturnFundFromEphemeralKeys returns funds from all ephemeral keys to the root key wallet
-// using a variefy of retry strategies.
-func ReturnFundFromEphemeralKeys(l zerolog.Logger, client *seth.Client) error {
-	err := client.NonceManager.UpdateNonces()
-	if err != nil {
-		return err
-	}
-	eg := errgroup.Group{}
-	for i := 1; i < len(client.Addresses); i++ {
-		i := i
-		eg.Go(func() error {
-			return sendAllFundsIfPossible(l, client, client.PrivateKeys[i])
-		})
-	}
-
-	return eg.Wait()
-}

integration-tests/testreporters/keeper_benchmark.go

@@ -223,8 +223,9 @@ func (k *KeeperBenchmarkTestReporter) WriteReport(folderLocation string) error {
 	k.Summary.Metrics.TotalTimesPerformed = totalPerformed
 	k.Summary.Metrics.TotalStaleReports = totalStaleReports
 	k.Summary.Metrics.PercentStale = pctStale
-	k.Summary.Metrics.AverageActualPerformsPerBlock = float64(totalPerformed) / float64(k.Summary.TestInputs["BlockRange"].(int64))
-
+	if k.Summary.TestInputs["BlockRange"] != nil {
+		k.Summary.Metrics.AverageActualPerformsPerBlock = float64(totalPerformed) / float64(k.Summary.TestInputs["BlockRange"].(int64))
+	}
 	// TODO: Set test expectations
 	/* Expect(int64(pctWithinSLA)).Should(BeNumerically(">=", int64(80)), "Expected PercentWithinSLA to be greater than or equal to 80, but got %f", pctWithinSLA)
 	Expect(int64(pctReverted)).Should(BeNumerically("<=", int64(10)), "Expected PercentRevert to be less than or equal to 10, but got %f", pctReverted)

integration-tests/testsetups/keeper_benchmark.go

@@ -252,21 +252,14 @@ func (k *KeeperBenchmarkTest) Run() {
 		}
 	}
 
-	effectiveBlockRange := inputs.Upkeeps.BlockRange + inputs.UpkeepSLA
 	k.log.Info().Msgf("Waiting for %d blocks for all upkeeps to be performed", inputs.Upkeeps.BlockRange+inputs.UpkeepSLA)
-	timeout := effectiveBlockRange * 3 * int64(time.Second)
-	timeout = int64(float64(timeout) * 1.1)
-	timeoutDuration := time.Duration(timeout)
 
-	ctx, cancel := context.WithDeadline(context.Background(), time.Now().Add(timeoutDuration))
-	defer cancel()
-	errgroup, errCtx := errgroup.WithContext(ctx)
+	errgroup, errCtx := errgroup.WithContext(context.Background())
 
 	var startedObservations = atomic.Int32{}
 	var finishedObservations = atomic.Int32{}
 
 	for rIndex := range k.keeperRegistries {
-		// headerCh := make(chan *types.Header, len(k.upkeepIDs[rIndex]))
 		for index, upkeepID := range k.upkeepIDs[rIndex] {
 			upkeepIDCopy := upkeepID
 			registryIndex := rIndex
@@ -275,7 +268,6 @@ func (k *KeeperBenchmarkTest) Run() {
 				startedObservations.Add(1)
 				k.log.Info().Str("UpkeepID", upkeepIDCopy.String()).Msg("Starting upkeep observation")
 
-				// doneCh := make(chan struct{})
 				confirmer := contracts.NewKeeperConsumerBenchmarkkUpkeepObserver(
 					k.keeperConsumerContracts[registryIndex],
 					k.keeperRegistries[registryIndex],
@@ -286,7 +278,6 @@ func (k *KeeperBenchmarkTest) Run() {
 					upkeepIndex,
 					inputs.Upkeeps.FirstEligibleBuffer,
 					k.log,
-					// doneCh,
 				)
 
 				k.log.Debug().Str("UpkeepID", upkeepIDCopy.String()).Msg("Subscribing to new headers for upkeep observation")
@@ -304,10 +295,6 @@ func (k *KeeperBenchmarkTest) Run() {
 						k.log.Error().Err(errCtx.Err()).Str("UpkeepID", upkeepIDCopy.String()).Msg("Stopping obervations due to error in one of the goroutines")
 						sub.Unsubscribe()
 						return nil
-					case <-ctx.Done(): // timeout, abandon ship!
-						k.log.Error().Str("UpkeepID", upkeepIDCopy.String()).Msg("Stopping obervations due to timeout")
-						sub.Unsubscribe()
-						return fmt.Errorf("failed to observe desired block range for upkeep %s before timeout", upkeepIDCopy.String())
 					case header := <-headerCh: // new block, check if upkeep was performed
 						finished, headerErr := confirmer.ReceiveHeader(header)
 						if headerErr != nil {

integration-tests/utils/seth.go

@@ -67,13 +67,25 @@ func MustReplaceSimulatedNetworkUrlWithK8(l zerolog.Logger, network blockchain.E
 		return network
 	}
 
-	if _, ok := testEnvironment.URLs["Simulated Geth"]; !ok {
+	networkKeys := []string{"Simulated Geth", "Simulated-Geth"}
+	var keyToUse string
+
+	for _, key := range networkKeys {
+		_, ok := testEnvironment.URLs[key]
+		if ok {
+			keyToUse = key
+			break
+		}
+	}
+
+	if keyToUse == "" {
 		for k := range testEnvironment.URLs {
 			l.Info().Str("Network", k).Msg("Available networks")
 		}
 		panic("no network settings for Simulated Geth")
 	}
-	network.URLs = testEnvironment.URLs["Simulated Geth"]
+
+	network.URLs = testEnvironment.URLs[keyToUse]
 
 	return network
 }