From fc9c0814d21a0fb692ac7d510fda0d334fa4e36e Mon Sep 17 00:00:00 2001 From: Silas Lenihan Date: Thu, 2 May 2024 18:07:49 -0400 Subject: [PATCH 1/8] Update finality depth check headtracker Signed-off-by: Silas Lenihan --- common/headtracker/head_tracker.go | 4 ++-- common/types/head.go | 3 +++ common/types/mocks/head.go | 20 +++++++++++++++++++ .../evm/headtracker/head_tracker_test.go | 1 + core/chains/evm/types/models.go | 1 + 5 files changed, 27 insertions(+), 2 deletions(-) diff --git a/common/headtracker/head_tracker.go b/common/headtracker/head_tracker.go index bc7a4910b39..2af17d56b33 100644 --- a/common/headtracker/head_tracker.go +++ b/common/headtracker/head_tracker.go @@ -250,8 +250,8 @@ func (ht *headTracker[HTH, S, ID, BLOCK_HASH]) handleNewHead(ctx context.Context } } else { ht.log.Debugw("Got out of order head", "blockNum", head.BlockNumber(), "head", head.BlockHash(), "prevHead", prevHead.BlockNumber()) - prevUnFinalizedHead := prevHead.BlockNumber() - int64(ht.config.FinalityDepth()) - if head.BlockNumber() < prevUnFinalizedHead { + fmt.Println("PREVHEAD:", prevHead, head.BlockNumber()) + if head.BlockNumber() <= prevHead.LatestFinalizedHead().BlockNumber() { promOldHead.WithLabelValues(ht.chainID.String()).Inc() ht.log.Criticalf("Got very old block with number %d (highest seen was %d). This is a problem and either means a very deep re-org occurred, one of the RPC nodes has gotten far out of sync, or the chain went backwards in block numbers. This node may not function correctly without manual intervention.", head.BlockNumber(), prevHead.BlockNumber()) ht.SvcErrBuffer.Append(errors.New("got very old block")) diff --git a/common/types/head.go b/common/types/head.go index 4ecdb981c78..9d927d4f5e4 100644 --- a/common/types/head.go +++ b/common/types/head.go @@ -38,4 +38,7 @@ type Head[BLOCK_HASH Hashable] interface { BlockDifficulty() *big.Int // IsValid returns true if the head is valid. IsValid() bool + + // Returns the latest finalized based on finality tag or depth + LatestFinalizedHead() Head[BLOCK_HASH] } diff --git a/common/types/mocks/head.go b/common/types/mocks/head.go index a8cbca07355..8fcd57a33c9 100644 --- a/common/types/mocks/head.go +++ b/common/types/mocks/head.go @@ -202,6 +202,26 @@ func (_m *Head[BLOCK_HASH]) IsValid() bool { return r0 } +// LatestFinalizedHead provides a mock function with given fields: +func (_m *Head[BLOCK_HASH]) LatestFinalizedHead() types.Head[BLOCK_HASH] { + ret := _m.Called() + + if len(ret) == 0 { + panic("no return value specified for LatestFinalizedHead") + } + + var r0 types.Head[BLOCK_HASH] + if rf, ok := ret.Get(0).(func() types.Head[BLOCK_HASH]); ok { + r0 = rf() + } else { + if ret.Get(0) != nil { + r0 = ret.Get(0).(types.Head[BLOCK_HASH]) + } + } + + return r0 +} + // NewHead creates a new instance of Head. It also registers a testing interface on the mock and a cleanup function to assert the mocks expectations. // The first argument is typically a *testing.T value. func NewHead[BLOCK_HASH types.Hashable](t interface { diff --git a/core/chains/evm/headtracker/head_tracker_test.go b/core/chains/evm/headtracker/head_tracker_test.go index b8bdb1f5703..654207e1a28 100644 --- a/core/chains/evm/headtracker/head_tracker_test.go +++ b/core/chains/evm/headtracker/head_tracker_test.go @@ -64,6 +64,7 @@ func TestHeadTracker_New(t *testing.T) { orm := headtracker.NewORM(cltest.FixtureChainID, db) assert.Nil(t, orm.IdempotentInsertHead(testutils.Context(t), cltest.Head(1))) last := cltest.Head(16) + last.IsFinalized = true assert.Nil(t, orm.IdempotentInsertHead(testutils.Context(t), last)) assert.Nil(t, orm.IdempotentInsertHead(testutils.Context(t), cltest.Head(10))) diff --git a/core/chains/evm/types/models.go b/core/chains/evm/types/models.go index 1bf47f84726..9895017bab2 100644 --- a/core/chains/evm/types/models.go +++ b/core/chains/evm/types/models.go @@ -169,6 +169,7 @@ func (h *Head) ChainHashes() []common.Hash { func (h *Head) LatestFinalizedHead() commontypes.Head[common.Hash] { for h != nil && !h.IsFinalized { + fmt.Println(h.BlockNumber(), h.IsFinalized) h = h.Parent } From b4a4e0ef89d2b381c8753848378c9dfae22e844f Mon Sep 17 00:00:00 2001 From: Silas Lenihan Date: Fri, 3 May 2024 10:03:20 -0400 Subject: [PATCH 2/8] added check for nil prevLatestFinalized --- common/headtracker/head_tracker.go | 5 +++-- core/chains/evm/types/models.go | 4 +++- 2 files changed, 6 insertions(+), 3 deletions(-) diff --git a/common/headtracker/head_tracker.go b/common/headtracker/head_tracker.go index 2af17d56b33..55172f2f059 100644 --- a/common/headtracker/head_tracker.go +++ b/common/headtracker/head_tracker.go @@ -250,8 +250,9 @@ func (ht *headTracker[HTH, S, ID, BLOCK_HASH]) handleNewHead(ctx context.Context } } else { ht.log.Debugw("Got out of order head", "blockNum", head.BlockNumber(), "head", head.BlockHash(), "prevHead", prevHead.BlockNumber()) - fmt.Println("PREVHEAD:", prevHead, head.BlockNumber()) - if head.BlockNumber() <= prevHead.LatestFinalizedHead().BlockNumber() { + prevLatestFinalized := prevHead.LatestFinalizedHead() + + if prevLatestFinalized == nil || head.BlockNumber() <= prevLatestFinalized.BlockNumber() { promOldHead.WithLabelValues(ht.chainID.String()).Inc() ht.log.Criticalf("Got very old block with number %d (highest seen was %d). This is a problem and either means a very deep re-org occurred, one of the RPC nodes has gotten far out of sync, or the chain went backwards in block numbers. This node may not function correctly without manual intervention.", head.BlockNumber(), prevHead.BlockNumber()) ht.SvcErrBuffer.Append(errors.New("got very old block")) diff --git a/core/chains/evm/types/models.go b/core/chains/evm/types/models.go index 9895017bab2..ae1bb31bf04 100644 --- a/core/chains/evm/types/models.go +++ b/core/chains/evm/types/models.go @@ -169,7 +169,9 @@ func (h *Head) ChainHashes() []common.Hash { func (h *Head) LatestFinalizedHead() commontypes.Head[common.Hash] { for h != nil && !h.IsFinalized { - fmt.Println(h.BlockNumber(), h.IsFinalized) + if h.Parent == nil { + return nil + } h = h.Parent } From a80a646638c96f8a0cd39b73897de07c0a29a6f0 Mon Sep 17 00:00:00 2001 From: Silas Lenihan Date: Fri, 3 May 2024 10:04:56 -0400 Subject: [PATCH 3/8] added changeset --- .changeset/witty-onions-talk.md | 5 +++++ 1 file changed, 5 insertions(+) create mode 100644 .changeset/witty-onions-talk.md diff --git a/.changeset/witty-onions-talk.md b/.changeset/witty-onions-talk.md new file mode 100644 index 00000000000..b28de48b089 --- /dev/null +++ b/.changeset/witty-onions-talk.md @@ -0,0 +1,5 @@ +--- +"chainlink": minor +--- + +Switched finality check in HeadTracker to use the underlying finality type From c20bfd0589dbd6a371d1cea4c1305ab843f8bc38 Mon Sep 17 00:00:00 2001 From: Silas Lenihan Date: Fri, 3 May 2024 10:08:13 -0400 Subject: [PATCH 4/8] updated changeset --- .changeset/witty-onions-talk.md | 2 +- core/chains/evm/headtracker/head_tracker_test.go | 1 - 2 files changed, 1 insertion(+), 2 deletions(-) diff --git a/.changeset/witty-onions-talk.md b/.changeset/witty-onions-talk.md index b28de48b089..ddb4e9dbcd3 100644 --- a/.changeset/witty-onions-talk.md +++ b/.changeset/witty-onions-talk.md @@ -2,4 +2,4 @@ "chainlink": minor --- -Switched finality check in HeadTracker to use the underlying finality type +#internal Switched finality check in HeadTracker to use the underlying finality type diff --git a/core/chains/evm/headtracker/head_tracker_test.go b/core/chains/evm/headtracker/head_tracker_test.go index 654207e1a28..b8bdb1f5703 100644 --- a/core/chains/evm/headtracker/head_tracker_test.go +++ b/core/chains/evm/headtracker/head_tracker_test.go @@ -64,7 +64,6 @@ func TestHeadTracker_New(t *testing.T) { orm := headtracker.NewORM(cltest.FixtureChainID, db) assert.Nil(t, orm.IdempotentInsertHead(testutils.Context(t), cltest.Head(1))) last := cltest.Head(16) - last.IsFinalized = true assert.Nil(t, orm.IdempotentInsertHead(testutils.Context(t), last)) assert.Nil(t, orm.IdempotentInsertHead(testutils.Context(t), cltest.Head(10))) From f8f41534d20e20e792fb2164de287ab221c0e713 Mon Sep 17 00:00:00 2001 From: Silas Lenihan Date: Wed, 8 May 2024 10:02:09 -0400 Subject: [PATCH 5/8] cleaned up nil protection in LatestFinalizedHead --- common/headtracker/head_tracker.go | 2 +- core/chains/evm/types/models.go | 8 ++++---- core/internal/cltest/cltest.go | 2 +- 3 files changed, 6 insertions(+), 6 deletions(-) diff --git a/common/headtracker/head_tracker.go b/common/headtracker/head_tracker.go index 55172f2f059..8ce5e198d47 100644 --- a/common/headtracker/head_tracker.go +++ b/common/headtracker/head_tracker.go @@ -252,7 +252,7 @@ func (ht *headTracker[HTH, S, ID, BLOCK_HASH]) handleNewHead(ctx context.Context ht.log.Debugw("Got out of order head", "blockNum", head.BlockNumber(), "head", head.BlockHash(), "prevHead", prevHead.BlockNumber()) prevLatestFinalized := prevHead.LatestFinalizedHead() - if prevLatestFinalized == nil || head.BlockNumber() <= prevLatestFinalized.BlockNumber() { + if prevLatestFinalized != nil && prevLatestFinalized.IsValid() && head.BlockNumber() <= prevLatestFinalized.BlockNumber() { promOldHead.WithLabelValues(ht.chainID.String()).Inc() ht.log.Criticalf("Got very old block with number %d (highest seen was %d). This is a problem and either means a very deep re-org occurred, one of the RPC nodes has gotten far out of sync, or the chain went backwards in block numbers. This node may not function correctly without manual intervention.", head.BlockNumber(), prevHead.BlockNumber()) ht.SvcErrBuffer.Append(errors.New("got very old block")) diff --git a/core/chains/evm/types/models.go b/core/chains/evm/types/models.go index ae1bb31bf04..a73a2014e01 100644 --- a/core/chains/evm/types/models.go +++ b/core/chains/evm/types/models.go @@ -168,14 +168,14 @@ func (h *Head) ChainHashes() []common.Hash { } func (h *Head) LatestFinalizedHead() commontypes.Head[common.Hash] { - for h != nil && !h.IsFinalized { - if h.Parent == nil { - return nil + for h != nil { + if h.IsFinalized { + return h } h = h.Parent } - return h + return nil } func (h *Head) ChainID() *big.Int { diff --git a/core/internal/cltest/cltest.go b/core/internal/cltest/cltest.go index 58cedbb96e1..cdc29786450 100644 --- a/core/internal/cltest/cltest.go +++ b/core/internal/cltest/cltest.go @@ -993,7 +993,7 @@ func AssertEthTxAttemptCountStays(t testing.TB, txStore txmgr.TestEvmTxStore, wa return txaIds } -// Head given the value convert it into an Head +// Head given the value convert it into a Head func Head(val interface{}) *evmtypes.Head { var h evmtypes.Head time := uint64(0) From 56dc83f11430aba4f8b07810a06cbe07a7a4c1e6 Mon Sep 17 00:00:00 2001 From: Silas Lenihan Date: Thu, 9 May 2024 10:07:36 -0400 Subject: [PATCH 6/8] Added error tuple to LatestFinalizedHead --- common/headtracker/head_tracker.go | 4 ++-- common/types/head.go | 2 +- common/types/mocks/head.go | 14 ++++++++++++-- core/chains/evm/types/models.go | 7 ++++--- 4 files changed, 19 insertions(+), 8 deletions(-) diff --git a/common/headtracker/head_tracker.go b/common/headtracker/head_tracker.go index 8ce5e198d47..695dc9f7481 100644 --- a/common/headtracker/head_tracker.go +++ b/common/headtracker/head_tracker.go @@ -250,9 +250,9 @@ func (ht *headTracker[HTH, S, ID, BLOCK_HASH]) handleNewHead(ctx context.Context } } else { ht.log.Debugw("Got out of order head", "blockNum", head.BlockNumber(), "head", head.BlockHash(), "prevHead", prevHead.BlockNumber()) - prevLatestFinalized := prevHead.LatestFinalizedHead() + prevLatestFinalized, err := prevHead.LatestFinalizedHead() - if prevLatestFinalized != nil && prevLatestFinalized.IsValid() && head.BlockNumber() <= prevLatestFinalized.BlockNumber() { + if err == nil && head.BlockNumber() <= prevLatestFinalized.BlockNumber() { promOldHead.WithLabelValues(ht.chainID.String()).Inc() ht.log.Criticalf("Got very old block with number %d (highest seen was %d). This is a problem and either means a very deep re-org occurred, one of the RPC nodes has gotten far out of sync, or the chain went backwards in block numbers. This node may not function correctly without manual intervention.", head.BlockNumber(), prevHead.BlockNumber()) ht.SvcErrBuffer.Append(errors.New("got very old block")) diff --git a/common/types/head.go b/common/types/head.go index 9d927d4f5e4..3b0955fdb5a 100644 --- a/common/types/head.go +++ b/common/types/head.go @@ -40,5 +40,5 @@ type Head[BLOCK_HASH Hashable] interface { IsValid() bool // Returns the latest finalized based on finality tag or depth - LatestFinalizedHead() Head[BLOCK_HASH] + LatestFinalizedHead() (Head[BLOCK_HASH], error) } diff --git a/common/types/mocks/head.go b/common/types/mocks/head.go index 8fcd57a33c9..172cf661d00 100644 --- a/common/types/mocks/head.go +++ b/common/types/mocks/head.go @@ -203,7 +203,7 @@ func (_m *Head[BLOCK_HASH]) IsValid() bool { } // LatestFinalizedHead provides a mock function with given fields: -func (_m *Head[BLOCK_HASH]) LatestFinalizedHead() types.Head[BLOCK_HASH] { +func (_m *Head[BLOCK_HASH]) LatestFinalizedHead() (types.Head[BLOCK_HASH], error) { ret := _m.Called() if len(ret) == 0 { @@ -211,6 +211,10 @@ func (_m *Head[BLOCK_HASH]) LatestFinalizedHead() types.Head[BLOCK_HASH] { } var r0 types.Head[BLOCK_HASH] + var r1 error + if rf, ok := ret.Get(0).(func() (types.Head[BLOCK_HASH], error)); ok { + return rf() + } if rf, ok := ret.Get(0).(func() types.Head[BLOCK_HASH]); ok { r0 = rf() } else { @@ -219,7 +223,13 @@ func (_m *Head[BLOCK_HASH]) LatestFinalizedHead() types.Head[BLOCK_HASH] { } } - return r0 + if rf, ok := ret.Get(1).(func() error); ok { + r1 = rf() + } else { + r1 = ret.Error(1) + } + + return r0, r1 } // NewHead creates a new instance of Head. It also registers a testing interface on the mock and a cleanup function to assert the mocks expectations. diff --git a/core/chains/evm/types/models.go b/core/chains/evm/types/models.go index acb916224bf..4d002db9c31 100644 --- a/core/chains/evm/types/models.go +++ b/core/chains/evm/types/models.go @@ -167,15 +167,16 @@ func (h *Head) ChainHashes() []common.Hash { return hashes } -func (h *Head) LatestFinalizedHead() commontypes.Head[common.Hash] { +func (h *Head) LatestFinalizedHead() (commontypes.Head[common.Hash], error) { for h != nil { if h.IsFinalized { - return h + return h, nil } + h = h.Parent } - return nil + return nil, pkgerrors.New("no finalized head") } func (h *Head) ChainID() *big.Int { From a84595fa43e134b549cb0872c89e3c21381ac28a Mon Sep 17 00:00:00 2001 From: Silas Lenihan Date: Thu, 9 May 2024 10:07:36 -0400 Subject: [PATCH 7/8] Added error tuple to LatestFinalizedHead --- core/chains/evm/types/head_test.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/core/chains/evm/types/head_test.go b/core/chains/evm/types/head_test.go index 97c536a3444..15d34dfc911 100644 --- a/core/chains/evm/types/head_test.go +++ b/core/chains/evm/types/head_test.go @@ -38,7 +38,7 @@ func TestHead_LatestFinalizedHead(t *testing.T) { for _, tc := range cases { t.Run(tc.Name, func(t *testing.T) { - actual := tc.Head.LatestFinalizedHead() + actual, _ := tc.Head.LatestFinalizedHead() if tc.Finalized == nil { assert.Nil(t, actual) } else { From 216c11ac2bc3bdfeb2a98b075e5de925e2a71f7b Mon Sep 17 00:00:00 2001 From: Silas Lenihan Date: Mon, 20 May 2024 10:55:04 -0400 Subject: [PATCH 8/8] removed error from LatestFinalizedHead --- common/headtracker/head_tracker.go | 4 ++-- common/types/head.go | 2 +- common/types/mocks/head.go | 16 +++------------- common/types/mocks/subscription.go | 2 +- core/chains/evm/types/head_test.go | 2 +- core/chains/evm/types/models.go | 6 +++--- 6 files changed, 11 insertions(+), 21 deletions(-) diff --git a/common/headtracker/head_tracker.go b/common/headtracker/head_tracker.go index 695dc9f7481..6247e87c673 100644 --- a/common/headtracker/head_tracker.go +++ b/common/headtracker/head_tracker.go @@ -250,9 +250,9 @@ func (ht *headTracker[HTH, S, ID, BLOCK_HASH]) handleNewHead(ctx context.Context } } else { ht.log.Debugw("Got out of order head", "blockNum", head.BlockNumber(), "head", head.BlockHash(), "prevHead", prevHead.BlockNumber()) - prevLatestFinalized, err := prevHead.LatestFinalizedHead() + prevLatestFinalized := prevHead.LatestFinalizedHead() - if err == nil && head.BlockNumber() <= prevLatestFinalized.BlockNumber() { + if prevLatestFinalized != nil && head.BlockNumber() <= prevLatestFinalized.BlockNumber() { promOldHead.WithLabelValues(ht.chainID.String()).Inc() ht.log.Criticalf("Got very old block with number %d (highest seen was %d). This is a problem and either means a very deep re-org occurred, one of the RPC nodes has gotten far out of sync, or the chain went backwards in block numbers. This node may not function correctly without manual intervention.", head.BlockNumber(), prevHead.BlockNumber()) ht.SvcErrBuffer.Append(errors.New("got very old block")) diff --git a/common/types/head.go b/common/types/head.go index 3b0955fdb5a..9d927d4f5e4 100644 --- a/common/types/head.go +++ b/common/types/head.go @@ -40,5 +40,5 @@ type Head[BLOCK_HASH Hashable] interface { IsValid() bool // Returns the latest finalized based on finality tag or depth - LatestFinalizedHead() (Head[BLOCK_HASH], error) + LatestFinalizedHead() Head[BLOCK_HASH] } diff --git a/common/types/mocks/head.go b/common/types/mocks/head.go index 172cf661d00..1768d57d175 100644 --- a/common/types/mocks/head.go +++ b/common/types/mocks/head.go @@ -1,4 +1,4 @@ -// Code generated by mockery v2.42.2. DO NOT EDIT. +// Code generated by mockery v2.38.0. DO NOT EDIT. package mocks @@ -203,7 +203,7 @@ func (_m *Head[BLOCK_HASH]) IsValid() bool { } // LatestFinalizedHead provides a mock function with given fields: -func (_m *Head[BLOCK_HASH]) LatestFinalizedHead() (types.Head[BLOCK_HASH], error) { +func (_m *Head[BLOCK_HASH]) LatestFinalizedHead() types.Head[BLOCK_HASH] { ret := _m.Called() if len(ret) == 0 { @@ -211,10 +211,6 @@ func (_m *Head[BLOCK_HASH]) LatestFinalizedHead() (types.Head[BLOCK_HASH], error } var r0 types.Head[BLOCK_HASH] - var r1 error - if rf, ok := ret.Get(0).(func() (types.Head[BLOCK_HASH], error)); ok { - return rf() - } if rf, ok := ret.Get(0).(func() types.Head[BLOCK_HASH]); ok { r0 = rf() } else { @@ -223,13 +219,7 @@ func (_m *Head[BLOCK_HASH]) LatestFinalizedHead() (types.Head[BLOCK_HASH], error } } - if rf, ok := ret.Get(1).(func() error); ok { - r1 = rf() - } else { - r1 = ret.Error(1) - } - - return r0, r1 + return r0 } // NewHead creates a new instance of Head. It also registers a testing interface on the mock and a cleanup function to assert the mocks expectations. diff --git a/common/types/mocks/subscription.go b/common/types/mocks/subscription.go index e52fafa7ca5..32db6dfa769 100644 --- a/common/types/mocks/subscription.go +++ b/common/types/mocks/subscription.go @@ -1,4 +1,4 @@ -// Code generated by mockery v2.42.2. DO NOT EDIT. +// Code generated by mockery v2.38.0. DO NOT EDIT. package mocks diff --git a/core/chains/evm/types/head_test.go b/core/chains/evm/types/head_test.go index 15d34dfc911..97c536a3444 100644 --- a/core/chains/evm/types/head_test.go +++ b/core/chains/evm/types/head_test.go @@ -38,7 +38,7 @@ func TestHead_LatestFinalizedHead(t *testing.T) { for _, tc := range cases { t.Run(tc.Name, func(t *testing.T) { - actual, _ := tc.Head.LatestFinalizedHead() + actual := tc.Head.LatestFinalizedHead() if tc.Finalized == nil { assert.Nil(t, actual) } else { diff --git a/core/chains/evm/types/models.go b/core/chains/evm/types/models.go index 4d002db9c31..2af5b81ccf8 100644 --- a/core/chains/evm/types/models.go +++ b/core/chains/evm/types/models.go @@ -167,16 +167,16 @@ func (h *Head) ChainHashes() []common.Hash { return hashes } -func (h *Head) LatestFinalizedHead() (commontypes.Head[common.Hash], error) { +func (h *Head) LatestFinalizedHead() commontypes.Head[common.Hash] { for h != nil { if h.IsFinalized { - return h, nil + return h } h = h.Parent } - return nil, pkgerrors.New("no finalized head") + return nil } func (h *Head) ChainID() *big.Int {