Merge branch 'develop' into verify-single-payment-token

.github/workflows/integration-tests.yml

@@ -88,15 +88,15 @@ jobs:
       - name: Check if image exists
         if: needs.changes.outputs.src == 'true'
         id: check-image
-        uses: smartcontractkit/chainlink-github-actions/docker/image-exists@00c6214deb10a3f374c6d3430c32c5202015d463 # v2.2.12
+        uses: smartcontractkit/chainlink-github-actions/docker/image-exists@eccde1970eca69f079d3efb3409938a72ade8497 # v2.2.13
         with:
           repository: chainlink
           tag: ${{ github.sha }}${{ matrix.image.tag-suffix }}
           AWS_REGION: ${{ secrets.QA_AWS_REGION }}
           AWS_ROLE_TO_ASSUME: ${{ secrets.QA_AWS_ROLE_TO_ASSUME }}
       - name: Build Image
         if: steps.check-image.outputs.exists == 'false' && needs.changes.outputs.src == 'true'
-        uses: smartcontractkit/chainlink-github-actions/chainlink-testing-framework/build-image@00c6214deb10a3f374c6d3430c32c5202015d463 # v2.2.12
+        uses: smartcontractkit/chainlink-github-actions/chainlink-testing-framework/build-image@eccde1970eca69f079d3efb3409938a72ade8497 # v2.2.13
         with:
           cl_repo: smartcontractkit/chainlink
           cl_ref: ${{ github.sha }}
@@ -205,7 +205,7 @@ jobs:
       ## Run this step when changes that require tests to be run are made
       - name: Run Tests
         if: needs.changes.outputs.src == 'true'
-        uses: smartcontractkit/chainlink-github-actions/chainlink-testing-framework/run-tests@00c6214deb10a3f374c6d3430c32c5202015d463 # v2.2.12
+        uses: smartcontractkit/chainlink-github-actions/chainlink-testing-framework/run-tests@eccde1970eca69f079d3efb3409938a72ade8497 # v2.2.13
         env:
           PYROSCOPE_SERVER: ${{ matrix.product.pyroscope_env == '' && '' || !startsWith(github.ref, 'refs/tags/') && '' || secrets.QA_PYROSCOPE_INSTANCE }} # Avoid sending blank envs https://github.com/orgs/community/discussions/25725
           PYROSCOPE_ENVIRONMENT: ${{ matrix.product.pyroscope_env }}
@@ -216,7 +216,7 @@ jobs:
           cl_repo: ${{ env.CHAINLINK_IMAGE }}
           cl_image_tag: ${{ github.sha }}
           aws_registries: ${{ secrets.QA_AWS_ACCOUNT_NUMBER }}
-          artifacts_location: ./integration-tests/smoke/logs
+          artifacts_location: ./integration-tests/smoke/logs/
           publish_check_name: ${{ matrix.product.name }}
           token: ${{ secrets.GITHUB_TOKEN }}
           go_mod_path: ./integration-tests/go.mod
@@ -312,7 +312,7 @@ jobs:
       ## Run this step when changes that require tests to be run are made
       - name: Run Tests
         if: needs.changes.outputs.src == 'true'
-        uses: smartcontractkit/chainlink-github-actions/chainlink-testing-framework/run-tests@00c6214deb10a3f374c6d3430c32c5202015d463 # v2.2.12
+        uses: smartcontractkit/chainlink-github-actions/chainlink-testing-framework/run-tests@eccde1970eca69f079d3efb3409938a72ade8497 # v2.2.13
         env:
           PYROSCOPE_SERVER: ${{ matrix.product.pyroscope_env == '' && '' || !startsWith(github.ref, 'refs/tags/') && '' || secrets.QA_PYROSCOPE_INSTANCE }} # Avoid sending blank envs https://github.com/orgs/community/discussions/25725
           PYROSCOPE_ENVIRONMENT: ${{ matrix.product.pyroscope_env }}
@@ -323,7 +323,8 @@ jobs:
           cl_repo: ${{ env.CHAINLINK_IMAGE }}
           cl_image_tag: ${{ github.sha }}
           aws_registries: ${{ secrets.QA_AWS_ACCOUNT_NUMBER }}
-          artifacts_location: ./integration-tests/smoke/logs
+          artifacts_name: ${{ matrix.product.name }}-test-logs
+          artifacts_location: ./integration-tests/smoke/logs/
           publish_check_name: ${{ matrix.product.name }}
           token: ${{ secrets.GITHUB_TOKEN }}
           go_mod_path: ./integration-tests/go.mod
@@ -335,7 +336,7 @@ jobs:
       ## Run this step when changes that do not need the test to run are made
       - name: Run Setup
         if: needs.changes.outputs.src == 'false'
-        uses: smartcontractkit/chainlink-github-actions/chainlink-testing-framework/setup-run-tests-environment@00c6214deb10a3f374c6d3430c32c5202015d463 # v2.2.12
+        uses: smartcontractkit/chainlink-github-actions/chainlink-testing-framework/setup-run-tests-environment@eccde1970eca69f079d3efb3409938a72ade8497 # v2.2.13
         with:
           test_download_vendor_packages_command: cd ./integration-tests && go mod download
           go_mod_path: ./integration-tests/go.mod
@@ -345,14 +346,6 @@ jobs:
           QA_AWS_ROLE_TO_ASSUME: ${{ secrets.QA_AWS_ROLE_TO_ASSUME }}
           QA_KUBECONFIG: ${{ secrets.QA_KUBECONFIG }}
 
-      - name: Upload test log
-        uses: actions/upload-artifact@0b7f8abb1508181956e8e162db84b466c27e18ce # v3.1.2
-        if: failure()
-        with:
-          name: test-log-${{ matrix.product.name }}
-          path: /tmp/gotest.log
-          retention-days: 7
-        continue-on-error: true
       - name: Collect Metrics
         if: always()
         id: collect-gha-metrics
@@ -425,7 +418,7 @@ jobs:
         with:
           ref: ${{ github.event.pull_request.head.sha || github.event.merge_group.head_sha }}
       - name: Run Setup
-        uses: smartcontractkit/chainlink-github-actions/chainlink-testing-framework/setup-go@00c6214deb10a3f374c6d3430c32c5202015d463 # v2.2.12
+        uses: smartcontractkit/chainlink-github-actions/chainlink-testing-framework/setup-go@eccde1970eca69f079d3efb3409938a72ade8497 # v2.2.13
         with:
           test_download_vendor_packages_command: |
             cd ./integration-tests
@@ -473,7 +466,7 @@ jobs:
         run: |
           echo "Running migration tests from version '${{ steps.get_latest_version.outputs.latest_version }}' to: '${{ github.sha }}'"
       - name: Run Migration Tests
-        uses: smartcontractkit/chainlink-github-actions/chainlink-testing-framework/run-tests@00c6214deb10a3f374c6d3430c32c5202015d463 # v2.2.12
+        uses: smartcontractkit/chainlink-github-actions/chainlink-testing-framework/run-tests@eccde1970eca69f079d3efb3409938a72ade8497 # v2.2.13
         with:
           test_command_to_run: make test_need_operator_assets && cd ./integration-tests && go test -timeout 30m -count=1 -json ./migration 2>&1 | tee /tmp/gotest.log | gotestfmt
           test_download_vendor_packages_command: cd ./integration-tests && go mod download
@@ -573,7 +566,7 @@ jobs:
     steps:
       - name: Check if image exists
         id: check-image
-        uses: smartcontractkit/chainlink-github-actions/docker/image-exists@00c6214deb10a3f374c6d3430c32c5202015d463 # v2.2.12
+        uses: smartcontractkit/chainlink-github-actions/docker/image-exists@eccde1970eca69f079d3efb3409938a72ade8497 # v2.2.13
         with:
           repository: chainlink-solana-tests
           tag: ${{ needs.get_solana_sha.outputs.sha }}
@@ -716,7 +709,7 @@ jobs:
           ref: ${{ needs.get_solana_sha.outputs.sha }}
       - name: Run Tests
         if: needs.changes.outputs.src == 'true'
-        uses: smartcontractkit/chainlink-github-actions/chainlink-testing-framework/run-tests@00c6214deb10a3f374c6d3430c32c5202015d463 # v2.2.12
+        uses: smartcontractkit/chainlink-github-actions/chainlink-testing-framework/run-tests@eccde1970eca69f079d3efb3409938a72ade8497 # v2.2.13
         with:
           test_command_to_run: export ENV_JOB_IMAGE=${{ secrets.QA_AWS_ACCOUNT_NUMBER }}.dkr.ecr.${{ secrets.QA_AWS_REGION }}.amazonaws.com/chainlink-solana-tests:${{ needs.get_solana_sha.outputs.sha }} && make test_smoke
           cl_repo: ${{ env.CHAINLINK_IMAGE }}
@@ -778,7 +771,7 @@ jobs:
           ref: ${{ github.event.pull_request.head.sha || github.event.merge_group.head_sha }}
       ## Only run OCR smoke test for now
       - name: Run Tests
-        uses: smartcontractkit/chainlink-github-actions/chainlink-testing-framework/run-tests@00c6214deb10a3f374c6d3430c32c5202015d463 # v2.2.12
+        uses: smartcontractkit/chainlink-github-actions/chainlink-testing-framework/run-tests@eccde1970eca69f079d3efb3409938a72ade8497 # v2.2.13
         env:
           PYROSCOPE_SERVER: ${{ secrets.QA_PYROSCOPE_INSTANCE }}
           PYROSCOPE_ENVIRONMENT: ci-smoke-ocr-evm-${{ matrix.testnet }} # TODO: Only for OCR for now

core/services/s4/errors.go

@@ -3,10 +3,11 @@ package s4
 import "errors"
 
 var (
-	ErrNotFound       = errors.New("not found")
-	ErrWrongSignature = errors.New("wrong signature")
-	ErrSlotIdTooBig   = errors.New("slot id is too big")
-	ErrPayloadTooBig  = errors.New("payload is too big")
-	ErrPastExpiration = errors.New("past expiration")
-	ErrVersionTooLow  = errors.New("version too low")
+	ErrNotFound          = errors.New("not found")
+	ErrWrongSignature    = errors.New("wrong signature")
+	ErrSlotIdTooBig      = errors.New("slot id is too big")
+	ErrPayloadTooBig     = errors.New("payload is too big")
+	ErrPastExpiration    = errors.New("past expiration")
+	ErrVersionTooLow     = errors.New("version too low")
+	ErrExpirationTooLong = errors.New("expiration too long")
 )

core/services/s4/storage.go

@@ -12,8 +12,9 @@ import (
 
 // Constraints specifies the global storage constraints.
 type Constraints struct {
-	MaxPayloadSizeBytes uint `json:"maxPayloadSizeBytes"`
-	MaxSlotsPerUser     uint `json:"maxSlotsPerUser"`
+	MaxPayloadSizeBytes    uint   `json:"maxPayloadSizeBytes"`
+	MaxSlotsPerUser        uint   `json:"maxSlotsPerUser"`
+	MaxExpirationLengthSec uint64 `json:"maxExpirationLengthSec"`
 }
 
 // Key identifies a versioned user record.
@@ -128,9 +129,13 @@ func (s *storage) Put(ctx context.Context, key *Key, record *Record, signature [
 	if len(record.Payload) > int(s.contraints.MaxPayloadSizeBytes) {
 		return ErrPayloadTooBig
 	}
-	if s.clock.Now().UnixMilli() > record.Expiration {
+	now := s.clock.Now().UnixMilli()
+	if now > record.Expiration {
 		return ErrPastExpiration
 	}
+	if record.Expiration-now > int64(s.contraints.MaxExpirationLengthSec)*1000 {
+		return ErrExpirationTooLong
+	}
 
 	envelope := NewEnvelopeFromRecord(key, record)
 	signer, err := envelope.GetSignerAddress(signature)

core/services/s4/storage_test.go

@@ -17,8 +17,9 @@ import (
 
 var (
 	constraints = s4.Constraints{
-		MaxSlotsPerUser:     5,
-		MaxPayloadSizeBytes: 32,
+		MaxSlotsPerUser:        5,
+		MaxPayloadSizeBytes:    32,
+		MaxExpirationLengthSec: 3600,
 	}
 )
 
@@ -100,6 +101,20 @@ func TestStorage_Errors(t *testing.T) {
 		assert.ErrorIs(t, err, s4.ErrPastExpiration)
 	})
 
+	t.Run("ErrExpirationTooLong", func(t *testing.T) {
+		key := &s4.Key{
+			Address: testutils.NewAddress(),
+			SlotId:  1,
+			Version: 0,
+		}
+		record := &s4.Record{
+			Payload:    make([]byte, 10),
+			Expiration: now.UnixMilli() + 10000000,
+		}
+		err := storage.Put(testutils.Context(t), key, record, []byte{})
+		assert.ErrorIs(t, err, s4.ErrExpirationTooLong)
+	})
+
 	t.Run("ErrWrongSignature", func(t *testing.T) {
 		privateKey, address := testutils.NewPrivateKeyAndAddress(t)
 		key := &s4.Key{

integration-tests/README.md

@@ -16,12 +16,26 @@ If you have previously run these smoke tests using GitHub Actions or some sort o
 
 See the [example.env](./example.env) file for environment variables you can set to configure things like network settings, Chainlink version, and log level. Remember to use `source .env` to activate your settings.
 
+## Build
+
+If you'd like to run the tests on a local build of Chainlink, you can point to your own docker image, or build a fresh one with `make`.
+
+`make build_docker_image image=<image-name> tag=<tag>`
+
+e.g.
+
+`make build_docker_image image=chainlink tag=test-tag`
+
 ## Run
 
 `go test ./smoke/<product>_test.go`
 
 It's generally recommended to run only one test at a time on a local machine as it needs a lot of docker containers and can peg your resources otherwise. You will see docker containers spin up on your machine for each component of the test where you can inspect logs.
 
+## Analyze
+
+You can see the results of each test in the terminal with normal `go test` output. If a test fails, logs of each Chainlink container will dump into the `smoke/logs/` folder for later analysis. You can also see these logs in CI uploaded as GitHub artifacts.
+
 ## Running Soak, Performance, Benchmark, and Chaos Tests
 
 These tests remain bound to a Kubernetes run environment, and require more complex setup and running instructions not documented here. We endeavor to make these easier to run and configure, but for the time being please seek a member of the QA/Test Tooling team if you want to run these.

integration-tests/docker/test_env/cl_node.go

@@ -152,8 +152,8 @@ func (n *ClNode) AddMercuryOCRJob(verifierAddr common.Address, fromBlock uint64,
 	}
 
 	bridges := utils.BuildBridges(eaUrls)
-	for _, b := range bridges {
-		err = n.API.MustCreateBridge(&b)
+	for index := range bridges {
+		err = n.API.MustCreateBridge(&bridges[index])
 		if err != nil {
 			return nil, err
 		}

integration-tests/docker/test_env/test_env.go

@@ -1,9 +1,15 @@
 package test_env
 
 import (
+	"context"
 	"encoding/json"
 	"fmt"
+	"io"
 	"math/big"
+	"os"
+	"path/filepath"
+	"testing"
+	"time"
 
 	"github.com/ethereum/go-ethereum/accounts/keystore"
 	"github.com/ethereum/go-ethereum/common"
@@ -16,6 +22,7 @@ import (
 	"github.com/smartcontractkit/chainlink-testing-framework/docker"
 	"github.com/smartcontractkit/chainlink-testing-framework/docker/test_env"
 	"github.com/smartcontractkit/chainlink-testing-framework/logwatch"
+	testUtils "github.com/smartcontractkit/chainlink-testing-framework/utils"
 	"github.com/smartcontractkit/chainlink/v2/core/services/chainlink"
 
 	"github.com/smartcontractkit/chainlink/integration-tests/client"
@@ -210,40 +217,84 @@ func (te *CLClusterTestEnv) Terminate() error {
 
 // Cleanup cleans the environment up after it's done being used, mainly for returning funds when on live networks.
 // Intended to be used as part of t.Cleanup() in tests.
-func (te *CLClusterTestEnv) Cleanup() error {
-	log.Info().Msg("Attempting to return Chainlink node funds to default network wallets")
+func (te *CLClusterTestEnv) Cleanup(t *testing.T) error {
+	l := testUtils.GetTestLogger(t)
 	if te.EVMClient == nil {
 		return errors.New("blockchain client is nil, unable to return funds from chainlink nodes")
 	}
 	if te.CLNodes == nil {
 		return errors.New("chainlink nodes are nil, unable to return funds from chainlink nodes")
 	}
+
+	// Check if we need to return funds
 	if te.EVMClient.NetworkSimulated() {
-		log.Info().Str("Network Name", te.EVMClient.GetNetworkName()).
+		l.Info().Str("Network Name", te.EVMClient.GetNetworkName()).
 			Msg("Network is a simulated network. Skipping fund return.")
+	} else {
+		l.Info().Msg("Attempting to return Chainlink node funds to default network wallets")
+		for _, chainlinkNode := range te.CLNodes {
+			fundedKeys, err := chainlinkNode.API.ExportEVMKeysForChain(te.EVMClient.GetChainID().String())
+			if err != nil {
+				return err
+			}
+			for _, key := range fundedKeys {
+				keyToDecrypt, err := json.Marshal(key)
+				if err != nil {
+					return err
+				}
+				// This can take up a good bit of RAM and time. When running on the remote-test-runner, this can lead to OOM
+				// issues. So we avoid running in parallel; slower, but safer.
+				decryptedKey, err := keystore.DecryptKey(keyToDecrypt, client.ChainlinkKeyPassword)
+				if err != nil {
+					return err
+				}
+				if err = te.EVMClient.ReturnFunds(decryptedKey.PrivateKey); err != nil {
+					return err
+				}
+			}
+		}
+	}
+
+	// TODO: This is an imperfect and temporary solution, see TT-590 for a more sustainable solution
+	// Collect logs if the test failed
+	if !t.Failed() {
 		return nil
 	}
 
-	for _, chainlinkNode := range te.CLNodes {
-		fundedKeys, err := chainlinkNode.API.ExportEVMKeysForChain(te.EVMClient.GetChainID().String())
-		if err != nil {
-			return err
-		}
-		for _, key := range fundedKeys {
-			keyToDecrypt, err := json.Marshal(key)
+	folder := fmt.Sprintf("./logs/%s-%s", t.Name(), time.Now().Format("2006-01-02T15-04-05"))
+	if err := os.MkdirAll(folder, os.ModePerm); err != nil {
+		return err
+	}
+
+	l.Warn().Msg("Test failed, collecting logs")
+	eg := &errgroup.Group{}
+	for _, n := range te.CLNodes {
+		node := n
+		eg.Go(func() error {
+			logFileName := filepath.Join(folder, fmt.Sprintf("node-%s.log", node.ContainerName))
+			logFile, err := os.OpenFile(logFileName, os.O_CREATE|os.O_WRONLY, 0644)
 			if err != nil {
 				return err
 			}
-			// This can take up a good bit of RAM and time. When running on the remote-test-runner, this can lead to OOM
-			// issues. So we avoid running in parallel; slower, but safer.
-			decryptedKey, err := keystore.DecryptKey(keyToDecrypt, client.ChainlinkKeyPassword)
+			defer logFile.Close()
+			logReader, err := node.Container.Logs(context.Background())
 			if err != nil {
 				return err
 			}
-			if err = te.EVMClient.ReturnFunds(decryptedKey.PrivateKey); err != nil {
+			_, err = io.Copy(logFile, logReader)
+			if err != nil {
 				return err
 			}
-		}
+			l.Info().Str("Node", node.ContainerName).Str("File", logFileName).Msg("Wrote Logs")
+			return nil
+		})
 	}
+
+	if err := eg.Wait(); err != nil {
+		return err
+	}
+
+	l.Info().Str("Logs Location", folder).Msg("Wrote Logs for Failed Test")
+
 	return nil
 }

integration-tests/smoke/cron_test.go

@@ -25,7 +25,7 @@ func TestCronBasic(t *testing.T) {
 		Build()
 	require.NoError(t, err)
 	t.Cleanup(func() {
-		if err := env.Cleanup(); err != nil {
+		if err := env.Cleanup(t); err != nil {
 			l.Error().Err(err).Msg("Error cleaning up test environment")
 		}
 	})

integration-tests/smoke/flux_test.go

@@ -31,7 +31,7 @@ func TestFluxBasic(t *testing.T) {
 		Build()
 	require.NoError(t, err)
 	t.Cleanup(func() {
-		if err := env.Cleanup(); err != nil {
+		if err := env.Cleanup(t); err != nil {
 			l.Error().Err(err).Msg("Error cleaning up test environment")
 		}
 	})

integration-tests/smoke/forwarder_ocr_test.go

@@ -27,7 +27,7 @@ func TestForwarderOCRBasic(t *testing.T) {
 		Build()
 	require.NoError(t, err)
 	t.Cleanup(func() {
-		if err := env.Cleanup(); err != nil {
+		if err := env.Cleanup(t); err != nil {
 			l.Error().Err(err).Msg("Error cleaning up test environment")
 		}
 	})

integration-tests/smoke/forwarders_ocr2_test.go

@@ -34,7 +34,7 @@ func TestForwarderOCR2Basic(t *testing.T) {
 		Build()
 	require.NoError(t, err)
 	t.Cleanup(func() {
-		if err := env.Cleanup(); err != nil {
+		if err := env.Cleanup(t); err != nil {
 			l.Error().Err(err).Msg("Error cleaning up test environment")
 		}
 	})