Merge branch 'develop' into ks-208/common-wf-graph

.changeset/early-shoes-sit.md

@@ -0,0 +1,10 @@
+---
+"chainlink": patch
+---
+
+Fixed CPU usage issues caused by inefficiencies in HeadTracker.
+
+HeadTracker's support of finality tags caused a drastic increase in the number of tracked blocks on the Arbitrum chain (from 50 to 12,000), which has led to a 30% increase in CPU usage. 
+
+The fix improves the data structure for tracking blocks and makes lookup more efficient. BenchmarkHeadTracker_Backfill shows 40x time reduction.
+#bugfix

.github/workflows/solidity.yml

@@ -114,22 +114,19 @@ jobs:
       run:
         working-directory: contracts
     needs: [changes]
+    if: needs.changes.outputs.changes == 'true'
     name: Solidity Lint
     runs-on: ubuntu-latest
     steps:
       - name: Checkout the repo
         uses: actions/checkout@9bb56186c3b09b4f86b1c65136769dd318469633 # v4.1.2
       - name: Setup NodeJS
-        if: needs.changes.outputs.changes == 'true'
         uses: ./.github/actions/setup-nodejs
       - name: Run pnpm lint
-        if: needs.changes.outputs.changes == 'true'
         run: pnpm lint
       - name: Run solhint
-        if: needs.changes.outputs.changes == 'true'
         run: pnpm solhint
       - name: Collect Metrics
-        if: needs.changes.outputs.changes == 'true'
         id: collect-gha-metrics
         uses: smartcontractkit/push-gha-metrics-action@dea9b546553cb4ca936607c2267a09c004e4ab3f # v3.0.0
         with:
@@ -145,19 +142,17 @@ jobs:
       run:
         working-directory: contracts
     needs: [changes]
+    if: needs.changes.outputs.changes == 'true'
     name: Prettier Formatting
     runs-on: ubuntu-latest
     steps:
       - name: Checkout the repo
         uses: actions/checkout@9bb56186c3b09b4f86b1c65136769dd318469633 # v4.1.2
       - name: Setup NodeJS
-        if: needs.changes.outputs.changes == 'true'
         uses: ./.github/actions/setup-nodejs
       - name: Run prettier check
-        if: needs.changes.outputs.changes == 'true'
         run: pnpm prettier:check
       - name: Collect Metrics
-        if: needs.changes.outputs.changes == 'true'
         id: collect-gha-metrics
         uses: smartcontractkit/push-gha-metrics-action@dea9b546553cb4ca936607c2267a09c004e4ab3f # v3.0.0
         with:
@@ -172,8 +167,8 @@ jobs:
     name: Publish Beta NPM
     environment: publish-contracts
     needs: [tag-check, changes, lint, prettier, native-compile, prepublish-test]
-    runs-on: ubuntu-latest
     if: needs.tag-check.outputs.is-pre-release == 'true'
+    runs-on: ubuntu-latest
     steps:
       - name: Checkout the repo
         uses: actions/checkout@b4ffde65f46336ab88eb53be808477a3936bae11 # v4.1.1
@@ -209,10 +204,10 @@ jobs:
     name: Publish Prod NPM
     environment: publish-contracts
     needs: [tag-check, changes, lint, prettier, native-compile, prepublish-test]
+    if: needs.tag-check.outputs.is-release == 'true'
     runs-on: ubuntu-latest
     permissions:
       contents: write
-    if: needs.tag-check.outputs.is-release == 'true'
     steps:
       - name: Checkout the repo
         uses: actions/checkout@b4ffde65f46336ab88eb53be808477a3936bae11 # v4.1.1

contracts/.changeset/seven-apes-drop.md

@@ -0,0 +1,5 @@
+---
+"@chainlink/contracts": patch
+---
+
+increase solhint max-warnings to 2 (from 0) to allow workflow to pass

contracts/package.json

@@ -18,7 +18,7 @@
     "prepublishOnly": "pnpm compile && ./scripts/prepublish_generate_abi_folder",
     "publish-beta": "pnpm publish --tag beta",
     "publish-prod": "pnpm publish --tag latest",
-    "solhint": "solhint --max-warnings 0 \"./src/v0.8/**/*.sol\""
+    "solhint": "solhint --max-warnings 2 \"./src/v0.8/**/*.sol\""
   },
   "files": [
     "src/v0.8",

core/chains/evm/headtracker/head_tracker_test.go

@@ -31,6 +31,7 @@ import (
 	"github.com/smartcontractkit/chainlink-common/pkg/utils/mailbox/mailboxtest"
 
 	htmocks "github.com/smartcontractkit/chainlink/v2/common/headtracker/mocks"
+	evmclient "github.com/smartcontractkit/chainlink/v2/core/chains/evm/client"
 	evmclimocks "github.com/smartcontractkit/chainlink/v2/core/chains/evm/client/mocks"
 	"github.com/smartcontractkit/chainlink/v2/core/chains/evm/headtracker"
 	httypes "github.com/smartcontractkit/chainlink/v2/core/chains/evm/headtracker/types"
@@ -983,7 +984,46 @@ func TestHeadTracker_Backfill(t *testing.T) {
 	})
 }
 
-func createHeadTracker(t *testing.T, ethClient *evmclimocks.Client, config headtracker.Config, htConfig headtracker.HeadTrackerConfig, orm headtracker.ORM) *headTrackerUniverse {
+// BenchmarkHeadTracker_Backfill - benchmarks HeadTracker's Backfill with focus on efficiency after initial
+// backfill on start up
+func BenchmarkHeadTracker_Backfill(b *testing.B) {
+	cfg := configtest.NewGeneralConfig(b, nil)
+
+	evmcfg := evmtest.NewChainScopedConfig(b, cfg)
+	db := pgtest.NewSqlxDB(b)
+	chainID := big.NewInt(evmclient.NullClientChainID)
+	orm := headtracker.NewORM(*chainID, db)
+	ethClient := evmclimocks.NewClient(b)
+	ethClient.On("ConfiguredChainID").Return(chainID)
+	ht := createHeadTracker(b, ethClient, evmcfg.EVM(), evmcfg.EVM().HeadTracker(), orm)
+	ctx := tests.Context(b)
+	makeHash := func(n int64) gethCommon.Hash {
+		return gethCommon.BigToHash(big.NewInt(n))
+	}
+	const finalityDepth = 12000 // observed value on Arbitrum
+	makeBlock := func(n int64) *evmtypes.Head {
+		return &evmtypes.Head{Number: n, Hash: makeHash(n), ParentHash: makeHash(n - 1)}
+	}
+	latest := makeBlock(finalityDepth)
+	finalized := makeBlock(1)
+	ethClient.On("HeadByHash", mock.Anything, mock.Anything).Return(func(_ context.Context, hash gethCommon.Hash) (*evmtypes.Head, error) {
+		number := hash.Big().Int64()
+		return makeBlock(number), nil
+	})
+	// run initial backfill to populate the database
+	err := ht.headTracker.Backfill(ctx, latest, finalized)
+	require.NoError(b, err)
+	b.ResetTimer()
+	// focus benchmark on processing of a new latest block
+	for i := 0; i < b.N; i++ {
+		latest = makeBlock(int64(finalityDepth + i))
+		finalized = makeBlock(int64(i + 1))
+		err := ht.headTracker.Backfill(ctx, latest, finalized)
+		require.NoError(b, err)
+	}
+}
+
+func createHeadTracker(t testing.TB, ethClient *evmclimocks.Client, config headtracker.Config, htConfig headtracker.HeadTrackerConfig, orm headtracker.ORM) *headTrackerUniverse {
 	lggr, ob := logger.TestObserved(t, zap.DebugLevel)
 	hb := headtracker.NewHeadBroadcaster(lggr)
 	hs := headtracker.NewHeadSaver(lggr, orm, config, htConfig)

core/chains/evm/headtracker/heads.go

@@ -26,8 +26,9 @@ type Heads interface {
 }
 
 type heads struct {
-	heads []*evmtypes.Head
-	mu    sync.RWMutex
+	heads    []*evmtypes.Head
+	headsMap map[common.Hash]*evmtypes.Head
+	mu       sync.RWMutex
 }
 
 func NewHeads() Heads {
@@ -48,12 +49,11 @@ func (h *heads) HeadByHash(hash common.Hash) *evmtypes.Head {
 	h.mu.RLock()
 	defer h.mu.RUnlock()
 
-	for _, head := range h.heads {
-		if head.Hash == hash {
-			return head
-		}
+	if h.headsMap == nil {
+		return nil
 	}
-	return nil
+
+	return h.headsMap[hash]
 }
 
 func (h *heads) Count() int {
@@ -74,26 +74,23 @@ func (h *heads) MarkFinalized(finalized common.Hash, minBlockToKeep int64) bool
 	}
 
 	// deep copy to avoid race on head.Parent
-	h.heads = deepCopy(h.heads, minBlockToKeep)
+	h.heads, h.headsMap = deepCopy(h.heads, minBlockToKeep)
 
-	head := h.heads[0]
-	foundFinalized := false
-	for head != nil {
-		if head.Hash == finalized {
-			foundFinalized = true
-		}
-
-		// we might see finalized to move back in chain due to request to lagging RPC,
-		// we should not override the flag in such cases
-		head.IsFinalized = head.IsFinalized || foundFinalized
-		head = head.Parent
+	finalizedHead, ok := h.headsMap[finalized]
+	if !ok {
+		return false
+	}
+	for finalizedHead != nil {
+		finalizedHead.IsFinalized = true
+		finalizedHead = finalizedHead.Parent
 	}
 
-	return foundFinalized
+	return true
 }
 
-func deepCopy(oldHeads []*evmtypes.Head, minBlockToKeep int64) []*evmtypes.Head {
+func deepCopy(oldHeads []*evmtypes.Head, minBlockToKeep int64) ([]*evmtypes.Head, map[common.Hash]*evmtypes.Head) {
 	headsMap := make(map[common.Hash]*evmtypes.Head, len(oldHeads))
+	heads := make([]*evmtypes.Head, 0, len(headsMap))
 	for _, head := range oldHeads {
 		if head.Hash == head.ParentHash {
 			// shouldn't happen but it is untrusted input
@@ -111,18 +108,11 @@ func deepCopy(oldHeads []*evmtypes.Head, minBlockToKeep int64) []*evmtypes.Head
 		// prefer head that was already in heads as it might have been marked as finalized on previous run
 		if _, ok := headsMap[head.Hash]; !ok {
 			headsMap[head.Hash] = &headCopy
+			heads = append(heads, &headCopy)
 		}
 	}
 
-	heads := make([]*evmtypes.Head, 0, len(headsMap))
-	// unsorted unique heads
-	{
-		for _, head := range headsMap {
-			heads = append(heads, head)
-		}
-	}
-
-	// sort the heads
+	// sort the heads as original slice might be out of order
 	sort.SliceStable(heads, func(i, j int) bool {
 		// sorting from the highest number to lowest
 		return heads[i].Number > heads[j].Number
@@ -137,13 +127,13 @@ func deepCopy(oldHeads []*evmtypes.Head, minBlockToKeep int64) []*evmtypes.Head
 		}
 	}
 
-	return heads
+	return heads, headsMap
 }
 
 func (h *heads) AddHeads(newHeads ...*evmtypes.Head) {
 	h.mu.Lock()
 	defer h.mu.Unlock()
 
 	// deep copy to avoid race on head.Parent
-	h.heads = deepCopy(append(h.heads, newHeads...), 0)
+	h.heads, h.headsMap = deepCopy(append(h.heads, newHeads...), 0)
 }

core/services/pipeline/task.divide_test.go

@@ -198,6 +198,7 @@ func TestDivideTask_Overflow(t *testing.T) {
 }
 
 func TestDivide_Example(t *testing.T) {
+	testutils.SkipFlakey(t, "BCF-3236")
 	t.Parallel()
 
 	dag := `