No new finalized Heads Implementation #13907

dhaidashenko · 2024-07-19T15:50:56Z

Added new health check that ensures RPC provides new finalized heads at least every NoNewFinalizedHeadsThreshold
Ensure that out-of-sync loop transitions to alive only when RPC satisfies all restrictions. (Previously it was sufficient for RPC to satisfies constraint that caused transition to out-of-sync)

Explanation of methodology to define NoNewFinalizedHeadsThreshold is available here
Default values were only set for chains that have FinalityTagEnabled = true for CCIP and NoNewHeadsThreshold > 0.

common/client/models.go

DylanTinianov

Small nit on sync issue naming, but looks good.

common/client/node_lifecycle.go

common/client/node_lifecycle_test.go

Co-authored-by: amit-momin <108959691+amit-momin@users.noreply.github.com>

…hub.com:smartcontractkit/chainlink into feature/BCI-3731-no-new-finalized-block-develop

amit-momin

LGTM

common/client/models.go

…evelop

dimriou · 2024-07-29T09:00:33Z

common/client/node_lifecycle.go

+	//n.stateMu.Lock()
+	//n.healthCheckSubs = append(n.healthCheckSubs, sub)
+	//n.stateMu.Unlock()


Are these placeholders?

Yes, after merging the BCI-2875 result.sub = sub will be replaced with

n.stateMu.Lock() n.healthCheckSubs = append(n.healthCheckSubs, sub) n.stateMu.Unlock()

dimriou · 2024-07-29T09:57:14Z

common/client/node_lifecycle.go

+			if !n.onNewHead(lggr, &localHighestChainInfo, head) {
+				continue
+			}
+


nit: Can you add a one-line comment here to quickly explain how we use bitwise operators? i.e. "We clear NoNewHead sync issue, after receiving a new head and check for out of sync." or something along these lines. It took me some time to figure out what is going on, without context.

dimriou · 2024-07-29T10:07:20Z

common/client/models.go

+type syncIssue int
+
+const (
+	// syncIssueNotInSyncWithPool - RPC is lagging behind the highest block observed within the pool of RPCs
+	syncIssueNotInSyncWithPool syncIssue = 1 << iota
+	// syncIssueNoNewHead - RPC failed to produce a new head for too long
+	syncIssueNoNewHead
+	// syncIssueNoNewFinalizedHead - RPC failed to produce a new finalized head for too long
+	syncIssueNoNewFinalizedHead
+	syncIssueLen
+)


Does changing syncIssue to syncStatus and adding synced state as 0 work, or does it mess up the enumeration incrementation? I noticed in outOfSyncLoop we use a 0 value instead of an enum value because of this.

…hub.com:smartcontractkit/chainlink into feature/BCI-3731-no-new-finalized-block-develop

cl-sonarqube-production · 2024-07-29T11:15:56Z

Quality Gate passed

Issues
7 New issues
1 Fixed issue
0 Accepted issues

Measures
0 Security Hotspots
91.0% Coverage on New Code
0.0% Duplication on New Code

See analysis details on SonarQube

* No new finalized Heads Implementation * fixed tests * update defaults for NoNewFinalizedHeadsThreshold * Update common/client/node_lifecycle.go Co-authored-by: amit-momin <108959691+amit-momin@users.noreply.github.com> * Update common/client/node_lifecycle_test.go Co-authored-by: amit-momin <108959691+amit-momin@users.noreply.github.com> * Update common/client/node_lifecycle_test.go Co-authored-by: amit-momin <108959691+amit-momin@users.noreply.github.com> * rename HeadIsNotIncreasing to NoNewHead * move and add docs for syncIssue consts * rename syncIssue to syncStatus --------- Co-authored-by: amit-momin <108959691+amit-momin@users.noreply.github.com>

## Motivation Cherry-pick of smartcontractkit/chainlink#14021 cherry-pick of smartcontractkit/chainlink#13957 cherry-pick of smartcontractkit/chainlink#13876 cherry-pick of smartcontractkit/chainlink#13907

* No new finalized Heads Implementation * fixed tests * update defaults for NoNewFinalizedHeadsThreshold * Update common/client/node_lifecycle.go Co-authored-by: amit-momin <108959691+amit-momin@users.noreply.github.com> * Update common/client/node_lifecycle_test.go Co-authored-by: amit-momin <108959691+amit-momin@users.noreply.github.com> * Update common/client/node_lifecycle_test.go Co-authored-by: amit-momin <108959691+amit-momin@users.noreply.github.com> * rename HeadIsNotIncreasing to NoNewHead * move and add docs for syncIssue consts * rename syncIssue to syncStatus --------- Co-authored-by: amit-momin <108959691+amit-momin@users.noreply.github.com>

No new finalized Heads Implementation

412ccc4

dhaidashenko temporarily deployed to sdlc July 19, 2024 15:51 — with GitHub Actions Inactive

fixed tests

3b0d090

dhaidashenko temporarily deployed to sdlc July 20, 2024 13:00 — with GitHub Actions Inactive

update defaults for NoNewFinalizedHeadsThreshold

84d4767

dhaidashenko temporarily deployed to sdlc July 21, 2024 14:47 — with GitHub Actions Inactive

dhaidashenko marked this pull request as ready for review July 21, 2024 15:11

dhaidashenko requested review from a team as code owners July 21, 2024 15:11

DylanTinianov reviewed Jul 24, 2024

View reviewed changes

common/client/models.go Outdated Show resolved Hide resolved

DylanTinianov previously approved these changes Jul 24, 2024

View reviewed changes

amit-momin reviewed Jul 24, 2024

View reviewed changes

common/client/node_lifecycle.go Outdated Show resolved Hide resolved

amit-momin reviewed Jul 24, 2024

View reviewed changes

common/client/node_lifecycle.go Show resolved Hide resolved

amit-momin reviewed Jul 24, 2024

View reviewed changes

common/client/node_lifecycle_test.go Outdated Show resolved Hide resolved

amit-momin reviewed Jul 24, 2024

View reviewed changes

common/client/node_lifecycle_test.go Outdated Show resolved Hide resolved

Update common/client/node_lifecycle.go

9ca1e34

Co-authored-by: amit-momin <108959691+amit-momin@users.noreply.github.com>

dhaidashenko dismissed DylanTinianov’s stale review via 9ca1e34 July 24, 2024 15:10

dhaidashenko temporarily deployed to sdlc July 24, 2024 15:10 — with GitHub Actions Inactive

Update common/client/node_lifecycle_test.go

ba7cdaf

Co-authored-by: amit-momin <108959691+amit-momin@users.noreply.github.com>

dhaidashenko temporarily deployed to sdlc July 24, 2024 15:22 — with GitHub Actions Inactive

Update common/client/node_lifecycle_test.go

43342e6

Co-authored-by: amit-momin <108959691+amit-momin@users.noreply.github.com>

dhaidashenko temporarily deployed to sdlc July 24, 2024 15:22 — with GitHub Actions Inactive

dhaidashenko added 2 commits July 24, 2024 17:29

rename HeadIsNotIncreasing to NoNewHead

e0bf9d5

Merge branch 'feature/BCI-3731-no-new-finalized-block-develop' of git…

023a73d

…hub.com:smartcontractkit/chainlink into feature/BCI-3731-no-new-finalized-block-develop

dhaidashenko temporarily deployed to sdlc July 24, 2024 15:31 — with GitHub Actions Inactive

dhaidashenko requested review from DylanTinianov and amit-momin July 24, 2024 16:02

amit-momin previously approved these changes Jul 24, 2024

View reviewed changes

dimriou reviewed Jul 25, 2024

View reviewed changes

common/client/models.go Outdated Show resolved Hide resolved

move and add docs for syncIssue consts

1dd0818

dhaidashenko temporarily deployed to sdlc July 28, 2024 09:29 — with GitHub Actions Inactive

dhaidashenko requested review from dimriou and amit-momin July 28, 2024 09:30

Merge branch 'develop' into feature/BCI-3731-no-new-finalized-block-d…

e0d3d5a

…evelop

dhaidashenko temporarily deployed to sdlc July 29, 2024 08:46 — with GitHub Actions Inactive

dimriou reviewed Jul 29, 2024

View reviewed changes

dhaidashenko requested a review from dimriou July 29, 2024 09:16

dimriou reviewed Jul 29, 2024

View reviewed changes

dhaidashenko added 2 commits July 29, 2024 12:47

rename syncIssue to syncStatus

82b2557

Merge branch 'feature/BCI-3731-no-new-finalized-block-develop' of git…

282f4e2

…hub.com:smartcontractkit/chainlink into feature/BCI-3731-no-new-finalized-block-develop

dhaidashenko temporarily deployed to sdlc July 29, 2024 10:49 — with GitHub Actions Inactive

dhaidashenko requested a review from dimriou July 29, 2024 11:24

dimriou approved these changes Jul 29, 2024

View reviewed changes

amit-momin approved these changes Jul 29, 2024

View reviewed changes

dimriou added this pull request to the merge queue Jul 30, 2024

Merged via the queue into develop with commit 1eaf5e0 Jul 30, 2024
115 checks passed

dimriou deleted the feature/BCI-3731-no-new-finalized-block-develop branch July 30, 2024 10:51

github-actions bot mentioned this pull request Jul 29, 2024

[DO NOT MERGE] Changeset Release Preview - v2.18.0 #13148

Draft

This was referenced Jul 31, 2024

[DO NOT MERGE] Changeset Release Preview - v2.16.0 EthanAnro/chainlink#2

Draft

[DO NOT MERGE] Changeset Release Preview - v2.15.0 gainssystems/chainlink#1

Draft

This was referenced Aug 18, 2024

[DO NOT MERGE] Changeset Release Preview - v2.16.0 picoinnetwork/chainlink#1

Draft

[DO NOT MERGE] Changeset Release Preview - v2.16.0 chaingence/chainlink#1

Draft

simsonraj mentioned this pull request Aug 19, 2024

Astar & Hedera changes cherry-picked smartcontractkit/ccip#1324

Merged

github-actions bot mentioned this pull request Aug 21, 2024

[DO NOT MERGE] Changeset Release Preview - v2.17.0 YoutacRandS-VA/chainlink#23

Draft

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

No new finalized Heads Implementation #13907

No new finalized Heads Implementation #13907

dhaidashenko commented Jul 19, 2024 •

edited

Loading

DylanTinianov left a comment

amit-momin left a comment

dimriou Jul 29, 2024 •

edited

Loading

dhaidashenko Jul 29, 2024

dimriou Jul 29, 2024 •

edited

Loading

dimriou Jul 29, 2024

cl-sonarqube-production bot commented Jul 29, 2024

No new finalized Heads Implementation #13907

No new finalized Heads Implementation #13907

Conversation

dhaidashenko commented Jul 19, 2024 • edited Loading

DylanTinianov left a comment

Choose a reason for hiding this comment

amit-momin left a comment

Choose a reason for hiding this comment

dimriou Jul 29, 2024 • edited Loading

Choose a reason for hiding this comment

dhaidashenko Jul 29, 2024

Choose a reason for hiding this comment

dimriou Jul 29, 2024 • edited Loading

Choose a reason for hiding this comment

dimriou Jul 29, 2024

Choose a reason for hiding this comment

cl-sonarqube-production bot commented Jul 29, 2024

Quality Gate passed

dhaidashenko commented Jul 19, 2024 •

edited

Loading

dimriou Jul 29, 2024 •

edited

Loading

dimriou Jul 29, 2024 •

edited

Loading