Adapter Changes and Naming

Implements some changes we found helpful with Optimism testing. Also improves the CI process, and includes some QoL changes.

.github/workflows/lint.yaml

@@ -1,4 +1,4 @@
-name: Linting and Static Analysis
+name: Linting
 on:
   push:
     branches: [main]
@@ -30,21 +30,3 @@ jobs:
       run: go list -json -deps ./... > go.list
     - name: Nancy
       uses: sonatype-nexus-community/nancy-github-action@main
-
-  codeQL:
-    name: CodeQL Checks
-    runs-on: ubuntu-latest
-    permissions:
-      security-events: write
-    steps:
-      - name: Checkout repository
-        uses: actions/checkout@v2
-      - name: Initialize CodeQL
-        uses: github/codeql-action/init@v1
-        with:
-          languages: go
-      - name: Autobuild
-        uses: github/codeql-action/autobuild@v1
-      - name: Perform CodeQL Analysis
-        uses: github/codeql-action/analyze@v1
-

.github/workflows/static-analysis.yaml

@@ -0,0 +1,23 @@
+name: Static Analysis
+on:
+  push:
+    branches: [main]
+  schedule:
+    - cron: '23 19 * * 4'
+jobs:
+  codeQL:
+    name: CodeQL Checks
+    runs-on: ubuntu-latest
+    permissions:
+      security-events: write
+    steps:
+      - name: Checkout repository
+        uses: actions/checkout@v2
+      - name: Initialize CodeQL
+        uses: github/codeql-action/init@v1
+        with:
+          languages: go
+      - name: Autobuild
+        uses: github/codeql-action/autobuild@v1
+      - name: Perform CodeQL Analysis
+        uses: github/codeql-action/analyze@v1

.golangci.yaml

@@ -1,18 +1,60 @@
 run:
+  concurrency: 4
   timeout: 3m
   skip-dirs:
-    - explorer
     - contracts/ethereum
-    - contracts
-issues:
-  exclude-use-default: false
+linters:
+  enable:
+    - exhaustive
+    - exportloopref
+    - revive
+    - goimports
+    - gosec
+    - rowserrcheck
+  disable:
+    # These are all considered deprecated: https://github.com/golangci/golangci-lint/issues/1841
+    - deadcode
+    - structcheck
+    - unused
+    - varcheck
 linters-settings:
+  exhaustive:
+    default-signifies-exhaustive: true
   revive:
+    confidence: 0.8
     rules:
       - name: exported
         severity: error
+      - name: blank-imports
+      - name: context-as-argument
+      - name: context-keys-type
       - name: dot-imports
-        disabled: true
-linters:
-  enable:
-    - revive
+      - name: error-return
+      - name: error-strings
+      - name: error-naming
+      - name: if-return
+      - name: increment-decrement
+      - name: var-declaration
+      - name: package-comments
+      - name: range
+      - name: receiver-naming
+      - name: time-naming
+      - name: indent-error-flow
+      - name: errorf
+      - name: empty-block
+      - name: superfluous-else
+      - name: unreachable-code
+      - name: redefines-builtin-id
+      - name: waitgroup-by-value
+      - name: unconditional-recursion
+      - name: struct-tag
+      - name: string-of-int
+      - name: range-val-in-closure
+      - name: modifies-value-receiver
+      - name: modifies-parameter
+      - name: identical-branches
+      - name: get-return
+      - name: defer
+      - name: constant-logical-expr
+      - name: bool-literal-in-expr
+      - name: atomic

Makefile

@@ -20,7 +20,7 @@ test_smoke:
 	ginkgo -v -r --junit-report=tests-smoke-report.xml --keep-going --trace --randomize-all --randomize-suites --progress $(args) ./suite/smoke 
 
 test_performance:
-	NETWORKS=ethereum_geth_performance,ethereum_geth_performance ginkgo -r --junit-report=tests-performance-report.xml --keep-going --trace --randomize-all --randomize-suites --progress $(args) ./suite/performance 
+	ginkgo -r -timeout=200h --junit-report=tests-performance-report.xml --keep-going --trace --randomize-all --randomize-suites --progress $(args) ./suite/performance 
 
 test_chaos:
-	NETWORKS=ethereum_geth_performance,ethereum_geth_performance ginkgo -r --junit-report=tests-chaos-report.xml --keep-going --trace --randomize-all --randomize-suites --progress $(args) ./suite/chaos 
+	ginkgo -r --junit-report=tests-chaos-report.xml --keep-going --trace --randomize-all --randomize-suites --progress $(args) ./suite/chaos 

README.md

@@ -38,10 +38,16 @@ See our [suite/smoke](suite/smoke) directory for quite a few examples of the fra
 
 ## Chainlink Values
 
-If you would like to change the Chainlink values that are used for environments, you can use JSON to squash them. Have a look over at our [helmenv](https://github.com/smartcontractkit/helmenv/tree/v1.0.5/charts/chainlink) chainlink charts to get a grasp of how things are structured. We'll be writing more on this later, but for now, you can squash values by providing a `CHARTS` environment variable.
-
-```sh
-CHARTS='{"chainlink": {"values": {"chainlink": {"image": {"version": "<version>"}}}}}' make test_smoke args="-nodes=5"
+If you would like to change the Chainlink values that are used for environments, you can use the `framework.yaml` file,
+or set environment variables that are all caps versions of the values found in the config file.
+
+```yaml
+# Specify the image and version of the chainlink image you want to run tests against. Leave blank for default.
+chainlink_image: 
+chainlink_version: 
+
+# Specify the image and version of the simulated geth image you want to run tests against. Leave blank for default.
+# Has no effect when running tests on networks other than the simulated geth instances.
+geth_image: 
+geth_version: 
 ```
-
-You can also use a file name, head over to the [./config](./config/README.md) directory to see how.

actions/actions.go

@@ -12,7 +12,10 @@ import (
 	"github.com/onsi/ginkgo/v2"
 	"github.com/smartcontractkit/helmenv/environment"
 
+	"github.com/rs/zerolog/log"
+	"github.com/smartcontractkit/integrations-framework/config"
 	"github.com/smartcontractkit/integrations-framework/contracts"
+	"github.com/smartcontractkit/integrations-framework/utils"
 
 	"github.com/ethereum/go-ethereum/common"
 	"github.com/pkg/errors"
@@ -153,6 +156,13 @@ func GetMockserverInitializerDataForOTPE(
 // TeardownSuite tears down networks/clients and environment and creates a logs folder for failed tests in the
 // specified path
 func TeardownSuite(env *environment.Environment, nets *client.Networks, logsFolderPath string) error {
+	fConf, err := config.LoadFrameworkConfig(filepath.Join(utils.ProjectRoot, "framework.yaml"))
+	if err != nil {
+		log.Fatal().
+			Str("Path", utils.ProjectRoot).
+			Msg("Failed to load config")
+		return err
+	}
 	if ginkgo.CurrentSpecReport().Failed() {
 		testFilename := strings.Split(ginkgo.CurrentSpecReport().FileName(), ".")[0]
 		_, testName := filepath.Split(testFilename)
@@ -166,6 +176,19 @@ func TeardownSuite(env *environment.Environment, nets *client.Networks, logsFold
 			return err
 		}
 	}
+	switch strings.ToUpper(fConf.KeepEnvironments) {
+	case "ALWAYS":
+		env.Persistent = true
+	case "ONFAIL":
+		if ginkgo.CurrentSpecReport().Failed() {
+			env.Persistent = true
+		}
+	case "NEVER":
+		env.Persistent = false
+	default:
+		log.Warn().Str("Invalid Keep Value", fConf.KeepEnvironments).
+			Msg("Invalid 'keep_environments' value, see the 'framework.yaml' file")
+	}
 	if !env.Config.Persistent {
 		if err := env.Teardown(); err != nil {
 			return err

actions/ocr.go

@@ -1,52 +1,49 @@
 package actions
 
+//revive:disable:dot-imports
 import (
 	"fmt"
+	"math/big"
+	"strings"
+	"time"
+
 	. "github.com/onsi/gomega"
 	uuid "github.com/satori/go.uuid"
 	"github.com/smartcontractkit/integrations-framework/client"
 	"github.com/smartcontractkit/integrations-framework/contracts"
-	"math/big"
-	"time"
 )
 
-// FundNodes funds all chainlink nodes
-func FundNodes(networks *client.Networks, chainlinkNodes []client.Chainlink) func() {
-	return func() {
-		txCost, err := networks.Default.EstimateCostForChainlinkOperations(200)
-		Expect(err).ShouldNot(HaveOccurred())
-		err = FundChainlinkNodes(chainlinkNodes, networks.Default, txCost)
-		Expect(err).ShouldNot(HaveOccurred())
-	}
-}
+// This actions file often returns functions, rather than just values. These are used as common test helpers, and are
+// handy to have returning as functions so that Ginkgo can use them in an aesthetically pleasing way.
 
 // DeployOCRContracts deploys and funds a certain number of offchain aggregator contracts
 func DeployOCRContracts(
-	ocrInstances []contracts.OffchainAggregator,
+	numberOfContracts int,
 	linkTokenContract contracts.LinkToken,
 	contractDeployer contracts.ContractDeployer,
 	chainlinkNodes []client.Chainlink,
 	networks *client.Networks,
-) func() {
-	return func() {
-		var err error
-		linkTokenContract, err = contractDeployer.DeployLinkTokenContract()
+) []contracts.OffchainAggregator {
+	ocrInstances := []contracts.OffchainAggregator{}
+	for i := 0; i < numberOfContracts; i++ {
+		ocrInstance, err := contractDeployer.DeployOffChainAggregator(
+			linkTokenContract.Address(),
+			contracts.DefaultOffChainAggregatorOptions(),
+		)
+		Expect(err).ShouldNot(HaveOccurred())
+		// Exclude the first node, which will be used as a bootstrapper
+		err = ocrInstance.SetConfig(
+			chainlinkNodes[1:],
+			contracts.DefaultOffChainAggregatorConfig(len(chainlinkNodes[1:])),
+		)
+		ocrInstances = append(ocrInstances, ocrInstance)
+		Expect(err).ShouldNot(HaveOccurred())
+		err = linkTokenContract.Transfer(ocrInstance.Address(), big.NewInt(2e18))
+		Expect(err).ShouldNot(HaveOccurred())
+		err = networks.Default.WaitForEvents()
 		Expect(err).ShouldNot(HaveOccurred())
-
-		for i := 0; i < len(ocrInstances); i++ {
-			ocrInstances[i], err = contractDeployer.DeployOffChainAggregator(linkTokenContract.Address(), contracts.DefaultOffChainAggregatorOptions())
-			Expect(err).ShouldNot(HaveOccurred())
-			err = ocrInstances[i].SetConfig(
-				chainlinkNodes[1:],
-				contracts.DefaultOffChainAggregatorConfig(len(chainlinkNodes[1:])),
-			)
-			Expect(err).ShouldNot(HaveOccurred())
-			err = linkTokenContract.Transfer(ocrInstances[i].Address(), big.NewInt(2e18))
-			Expect(err).ShouldNot(HaveOccurred())
-			err = networks.Default.WaitForEvents()
-			Expect(err).ShouldNot(HaveOccurred())
-		}
 	}
+	return ocrInstances
 }
 
 // CreateOCRJobs bootstraps the first node and to the other nodes sends ocr jobs that
@@ -57,14 +54,14 @@ func CreateOCRJobs(
 	mockserver *client.MockserverClient,
 ) func() {
 	return func() {
-		for i := 0; i < len(ocrInstances); i++ {
+		for _, ocrInstance := range ocrInstances {
 			bootstrapNode := chainlinkNodes[0]
 			bootstrapP2PIds, err := bootstrapNode.ReadP2PKeys()
 			Expect(err).ShouldNot(HaveOccurred())
 			bootstrapP2PId := bootstrapP2PIds.Data[0].Attributes.PeerID
 			bootstrapSpec := &client.OCRBootstrapJobSpec{
 				Name:            fmt.Sprintf("bootstrap-%s", uuid.NewV4().String()),
-				ContractAddress: ocrInstances[i].Address(),
+				ContractAddress: ocrInstance.Address(),
 				P2PPeerID:       bootstrapP2PId,
 				IsBootstrapPeer: true,
 			}
@@ -81,16 +78,21 @@ func CreateOCRJobs(
 				Expect(err).ShouldNot(HaveOccurred())
 				nodeOCRKeyId := nodeOCRKeys.Data[0].ID
 
+				nodeContractPairID := buildNodeContractPairID(chainlinkNodes[nodeIndex], ocrInstance)
+				Expect(err).ShouldNot(HaveOccurred())
 				bta := client.BridgeTypeAttributes{
-					Name: fmt.Sprintf("node_%d_contract_%d", nodeIndex, i),
-					URL:  fmt.Sprintf("%s/node_%d_contract_%d", mockserver.Config.ClusterURL, nodeIndex, i),
+					Name: nodeContractPairID,
+					URL:  fmt.Sprintf("%s/%s", mockserver.Config.ClusterURL, nodeContractPairID),
 				}
 
+				// This sets a default value for all node and ocr instances in order to avoid 404 issues
+				SetAllAdapterResponses(0, ocrInstances, chainlinkNodes, mockserver)
+
 				err = chainlinkNodes[nodeIndex].CreateBridge(&bta)
 				Expect(err).ShouldNot(HaveOccurred())
 
 				ocrSpec := &client.OCRTaskJobSpec{
-					ContractAddress:    ocrInstances[i].Address(),
+					ContractAddress:    ocrInstance.Address(),
 					P2PPeerID:          nodeP2PId,
 					P2PBootstrapPeers:  []client.Chainlink{bootstrapNode},
 					KeyBundleID:        nodeOCRKeyId,
@@ -104,24 +106,33 @@ func CreateOCRJobs(
 	}
 }
 
-// SetAdapterResponses sets the mock responses in mockserver that are read by chainlink nodes
-// to simulate different adapters, to be used in combination with CreateOCRJobs
-func SetAdapterResponses(
-	results []int,
+// SetAdapterResponse sets a single adapter response that correlates with an ocr contract and a chainlink node
+func SetAdapterResponse(
+	response int,
+	ocrInstance contracts.OffchainAggregator,
+	chainlinkNode client.Chainlink,
+	mockserver *client.MockserverClient,
+) func() {
+	return func() {
+		nodeContractPairID := buildNodeContractPairID(chainlinkNode, ocrInstance)
+		path := fmt.Sprintf("/%s", nodeContractPairID)
+		err := mockserver.SetValuePath(path, response)
+		Expect(err).ShouldNot(HaveOccurred())
+	}
+}
+
+// SetAllAdapterResponses sets the mock responses in mockserver that are read by chainlink nodes
+// to simulate different adapters. This sets all adapter responses for each node and contract to the same response
+func SetAllAdapterResponses(
+	response int,
 	ocrInstances []contracts.OffchainAggregator,
 	chainlinkNodes []client.Chainlink,
 	mockserver *client.MockserverClient,
 ) func() {
 	return func() {
-		Expect(len(results)).Should(BeNumerically("==", len(chainlinkNodes[1:])))
-		for OCRInstanceIndex := range ocrInstances {
-			for nodeIndex := 1; nodeIndex < len(chainlinkNodes); nodeIndex++ {
-				path := fmt.Sprintf("/node_%d_contract_%d", nodeIndex, OCRInstanceIndex)
-				pathSelector := client.PathSelector{Path: path}
-				err := mockserver.ClearExpectation(pathSelector)
-				Expect(err).ShouldNot(HaveOccurred())
-				err = mockserver.SetValuePath(path, results[nodeIndex-1])
-				Expect(err).ShouldNot(HaveOccurred())
+		for _, ocrInstance := range ocrInstances {
+			for _, node := range chainlinkNodes {
+				SetAdapterResponse(response, ocrInstance, node, mockserver)()
 			}
 		}
 	}
@@ -145,3 +156,13 @@ func StartNewRound(
 		}
 	}
 }
+
+func buildNodeContractPairID(node client.Chainlink, ocrInstance contracts.OffchainAggregator) string {
+	Expect(node).ShouldNot(BeNil())
+	Expect(ocrInstance).ShouldNot(BeNil())
+	nodeAddress, err := node.PrimaryEthAddress()
+	Expect(err).ShouldNot(HaveOccurred())
+	shortNodeAddr := nodeAddress[2:12]
+	shortOCRAddr := ocrInstance.Address()[2:12]
+	return strings.ToLower(fmt.Sprintf("node_%s_contract_%s", shortNodeAddr, shortOCRAddr))
+}