Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

BCFR-1086 return finality violation error in health report #15548

Merged
merged 9 commits into from
Dec 13, 2024
Merged

BCFR-1086 return finality violation error in health report #15548

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
9 commits
Select commit Hold shift + click to select a range
0941519
return finality violation error in health report
dhaidashenko Dec 6, 2024
d99a2c3
gomod
dhaidashenko Dec 6, 2024
af23389
HealthReport tests
dhaidashenko Dec 11, 2024
0ef6201
propagate headtracker's errors
dhaidashenko Dec 11, 2024
baae23f
Merge branch 'develop' into feature/BCFR-1086-finality-violation
dhaidashenko Dec 11, 2024
2a5749f
bump chainlink-common
dhaidashenko Dec 11, 2024
3c77161
Merge branch 'develop' into feature/BCFR-1086-finality-violation
dhaidashenko Dec 11, 2024
ab8add5
Merge branch 'develop' into feature/BCFR-1086-finality-violation
dhaidashenko Dec 12, 2024
ab39c5b
Merge branch 'develop' into feature/BCFR-1086-finality-violation
dhaidashenko Dec 13, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 4 additions & 2 deletions core/chains/evm/logpoller/log_poller.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,8 @@ import (
pkgerrors "github.com/pkg/errors"
"golang.org/x/exp/maps"

commontypes "github.com/smartcontractkit/chainlink-common/pkg/types"

"github.com/smartcontractkit/chainlink-common/pkg/logger"
"github.com/smartcontractkit/chainlink-common/pkg/services"
"github.com/smartcontractkit/chainlink-common/pkg/timeutil"
Expand Down Expand Up @@ -91,6 +93,7 @@ type Client interface {
}

type HeadTracker interface {
services.Service
LatestAndFinalizedBlock(ctx context.Context) (latest, finalized *evmtypes.Head, err error)
}

Expand All @@ -99,7 +102,6 @@ var (
ErrReplayRequestAborted = pkgerrors.New("aborted, replay request cancelled")
ErrReplayInProgress = pkgerrors.New("replay request cancelled, but replay is already in progress")
ErrLogPollerShutdown = pkgerrors.New("replay aborted due to log poller shutdown")
ErrFinalityViolated = pkgerrors.New("finality violated")
)

type logPoller struct {
Expand Down Expand Up @@ -525,7 +527,7 @@ func (lp *logPoller) Close() error {

func (lp *logPoller) Healthy() error {
if lp.finalityViolated.Load() {
return ErrFinalityViolated
return commontypes.ErrFinalityViolated
}
return nil
}
Expand Down
9 changes: 7 additions & 2 deletions core/chains/evm/logpoller/log_poller_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -22,10 +22,13 @@ import (
"github.com/stretchr/testify/require"
"go.uber.org/zap/zapcore"

commontypes "github.com/smartcontractkit/chainlink-common/pkg/types"

"github.com/smartcontractkit/chainlink-common/pkg/logger"
"github.com/smartcontractkit/chainlink-common/pkg/types/query"
"github.com/smartcontractkit/chainlink-common/pkg/types/query/primitives"
commonutils "github.com/smartcontractkit/chainlink-common/pkg/utils"

"github.com/smartcontractkit/chainlink/v2/core/chains/evm/config/chaintype"

htMocks "github.com/smartcontractkit/chainlink/v2/common/headtracker/mocks"
Expand Down Expand Up @@ -1106,7 +1109,8 @@ func TestLogPoller_ReorgDeeperThanFinality(t *testing.T) {

secondPoll := th.PollAndSaveLogs(testutils.Context(t), firstPoll)
assert.Equal(t, firstPoll, secondPoll)
assert.Equal(t, logpoller.ErrFinalityViolated, th.LogPoller.Healthy())
require.Equal(t, commontypes.ErrFinalityViolated, th.LogPoller.Healthy())
require.Equal(t, commontypes.ErrFinalityViolated, th.LogPoller.HealthReport()[th.LogPoller.Name()])

// Manually remove re-org'd chain from the log poller to bring it back to life
// LogPoller should be healthy again after first poll
Expand All @@ -1116,7 +1120,8 @@ func TestLogPoller_ReorgDeeperThanFinality(t *testing.T) {
// Poll from latest
recoveryPoll := th.PollAndSaveLogs(testutils.Context(t), 1)
assert.Equal(t, int64(35), recoveryPoll)
assert.NoError(t, th.LogPoller.Healthy())
require.NoError(t, th.LogPoller.Healthy())
require.NoError(t, th.LogPoller.HealthReport()[th.LogPoller.Name()])
})
}
}
Expand Down
20 changes: 20 additions & 0 deletions core/services/relay/evm/chain_components_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ package evm_test
import (
"context"
"crypto/ecdsa"
"errors"
"fmt"
"math"
"math/big"
Expand All @@ -12,22 +13,28 @@ import (
"time"

"github.com/ethereum/go-ethereum/accounts/abi/bind"
"github.com/ethereum/go-ethereum/common"
evmtypes "github.com/ethereum/go-ethereum/core/types"
"github.com/ethereum/go-ethereum/crypto"
"github.com/ethereum/go-ethereum/ethclient/simulated"
"github.com/jmoiron/sqlx"
"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"

"github.com/smartcontractkit/chainlink-common/pkg/services"

commonconfig "github.com/smartcontractkit/chainlink-common/pkg/config"
commontestutils "github.com/smartcontractkit/chainlink-common/pkg/loop/testutils"
clcommontypes "github.com/smartcontractkit/chainlink-common/pkg/types"
"github.com/smartcontractkit/chainlink-common/pkg/types/interfacetests"

htMocks "github.com/smartcontractkit/chainlink/v2/common/headtracker/mocks"
"github.com/smartcontractkit/chainlink/v2/core/chains/evm/assets"
"github.com/smartcontractkit/chainlink/v2/core/chains/evm/client"
"github.com/smartcontractkit/chainlink/v2/core/chains/evm/logpoller"
lpMocks "github.com/smartcontractkit/chainlink/v2/core/chains/evm/logpoller/mocks"
evmtxmgr "github.com/smartcontractkit/chainlink/v2/core/chains/evm/txmgr"
clevmtypes "github.com/smartcontractkit/chainlink/v2/core/chains/evm/types"
"github.com/smartcontractkit/chainlink/v2/core/internal/cltest"
"github.com/smartcontractkit/chainlink/v2/core/internal/testutils"
"github.com/smartcontractkit/chainlink/v2/core/internal/testutils/configtest"
Expand Down Expand Up @@ -206,6 +213,19 @@ func TestContractReaderEventsInitValidation(t *testing.T) {
}
}

func TestChainReader_HealthReport(t *testing.T) {
lp := lpMocks.NewLogPoller(t)
lp.EXPECT().HealthReport().Return(map[string]error{"lp_name": clcommontypes.ErrFinalityViolated}).Once()
ht := htMocks.NewHeadTracker[*clevmtypes.Head, common.Hash](t)
htError := errors.New("head tracker error")
ht.EXPECT().HealthReport().Return(map[string]error{"ht_name": htError}).Once()
cr, err := evm.NewChainReaderService(testutils.Context(t), logger.NullLogger, lp, ht, nil, types.ChainReaderConfig{Contracts: nil})
require.NoError(t, err)
healthReport := cr.HealthReport()
require.True(t, services.ContainsError(healthReport, clcommontypes.ErrFinalityViolated), "expected chain reader to propagate logpoller's error")
require.True(t, services.ContainsError(healthReport, htError), "expected chain reader to propagate headtracker's error")
}

func TestChainComponents(t *testing.T) {
testutils.SkipFlakey(t, "https://smartcontract-it.atlassian.net/browse/BCFR-1083")
t.Parallel()
Expand Down
9 changes: 8 additions & 1 deletion core/services/relay/evm/chain_reader.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@ import (
"github.com/smartcontractkit/chainlink-common/pkg/types/query"
"github.com/smartcontractkit/chainlink-common/pkg/types/query/primitives"
"github.com/smartcontractkit/chainlink-common/pkg/values"

evmclient "github.com/smartcontractkit/chainlink/v2/core/chains/evm/client"
"github.com/smartcontractkit/chainlink/v2/core/chains/evm/logpoller"
evmtypes "github.com/smartcontractkit/chainlink/v2/core/chains/evm/types"
Expand Down Expand Up @@ -179,7 +180,13 @@ func (cr *chainReader) Close() error {
func (cr *chainReader) Ready() error { return nil }

func (cr *chainReader) HealthReport() map[string]error {
return map[string]error{cr.Name(): nil}
report := map[string]error{
cr.Name(): cr.Healthy(),
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ideally the CR Healthy() implementation should append all reports from underlying components like ht, lp, client, etc.
But I assume for now you are only worried about making FV work.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good point. Initially, I thought HT and the Client did not support reports, but it turns out that I've checked HT's interface instead of implementation.
Added propagation of HT reports. Created aticket to also implement it for client.

}

commonservices.CopyHealth(report, cr.lp.HealthReport())
commonservices.CopyHealth(report, cr.ht.HealthReport())
return report
}

func (cr *chainReader) Bind(ctx context.Context, bindings []commontypes.BoundContract) error {
Expand Down
Loading