Skip to content

CCIP-9669 finality checker metrics interface#1024

Merged
bukata-sa merged 2 commits intomainfrom
ccip-9669/ccip_o11y_finality_violation
Apr 21, 2026
Merged

CCIP-9669 finality checker metrics interface#1024
bukata-sa merged 2 commits intomainfrom
ccip-9669/ccip_o11y_finality_violation

Conversation

@bukata-sa
Copy link
Copy Markdown
Collaborator

@bukata-sa bukata-sa commented Apr 15, 2026

Refactor metric interfaces in executor and verifier to tighten ownership and separate checker-specific metrics.

Executor

  • MetricLabeler now embeds ccvcommon.CurseCheckerMetrics.
  • Removed the curse-related methods that were previously inlined on MetricLabeler.

Verifier

  • Added a new FinalityCheckerMetrics interface for finality violation metrics.
  • MetricLabeler now embeds both FinalityCheckerMetrics and common.CurseCheckerMetrics.
  • Moved the relevant methods out of MetricLabeler and into the dedicated interfaces.

Finality checker service

  • FinalityViolationCheckerService now depends on FinalityCheckerMetrics instead of the broader MetricLabeler.
  • Updated the constructor to match the new interface boundary.

Shared verifier interfaces

  • Exported FinalityCheckerMetrics from verifier/pkg

@bukata-sa bukata-sa force-pushed the ccip-9669/ccip_o11y_finality_violation branch from 4e44e15 to 46e3075 Compare April 15, 2026 14:32
@bukata-sa bukata-sa force-pushed the ccip-9669/ccip_o11y_finality_violation branch from 46e3075 to 5ea8fb6 Compare April 17, 2026 15:07
@bukata-sa bukata-sa marked this pull request as ready for review April 21, 2026 14:46
@bukata-sa bukata-sa requested review from a team and skudasov as code owners April 21, 2026 14:46
Copilot AI review requested due to automatic review settings April 21, 2026 14:46
@github-actions
Copy link
Copy Markdown

Code coverage report:

Package main ccip-9669/ccip_o11y_finality_violation diff
github.com/smartcontractkit/chainlink-ccv/aggregator 48.46% 48.46% +0.00%
github.com/smartcontractkit/chainlink-ccv/bootstrap 42.60% 42.60% +0.00%
github.com/smartcontractkit/chainlink-ccv/cli 65.13% 65.13% +0.00%
github.com/smartcontractkit/chainlink-ccv/cmd 0.00% 0.00% +0.00%
github.com/smartcontractkit/chainlink-ccv/common 50.74% 50.74% +0.00%
github.com/smartcontractkit/chainlink-ccv/executor 45.74% 45.74% +0.00%
github.com/smartcontractkit/chainlink-ccv/indexer 37.50% 37.46% -0.04%
github.com/smartcontractkit/chainlink-ccv/integration 47.68% 47.68% +0.00%
github.com/smartcontractkit/chainlink-ccv/pkg 100.00% 100.00% +0.00%
github.com/smartcontractkit/chainlink-ccv/pricer 0.00% 0.00% +0.00%
github.com/smartcontractkit/chainlink-ccv/protocol 65.19% 65.19% +0.00%
github.com/smartcontractkit/chainlink-ccv/verifier 32.55% 32.56% +0.01%

Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

Refactors verifier/executor metrics interfaces to better separate ownership and narrow dependencies, especially around curse-checker and finality-checker metrics.

Changes:

  • Verifier: Introduces FinalityCheckerMetrics and embeds it (plus common.CurseCheckerMetrics) into MetricLabeler.
  • Finality checker: Narrows dependency from MetricLabeler to FinalityCheckerMetrics.
  • Shared verifier interfaces: Re-exports FinalityCheckerMetrics via verifier/pkg.

Reviewed changes

Copilot reviewed 4 out of 4 changed files in this pull request and generated no comments.

File Description
verifier/pkg/vtypes/interfaces.go Adds FinalityCheckerMetrics; updates MetricLabeler to embed finality + curse metrics interfaces.
verifier/pkg/sourcereader/finality_checker.go Changes finality checker service dependency to FinalityCheckerMetrics.
verifier/pkg/interfaces.go Re-exports FinalityCheckerMetrics for external consumers.
executor/interfaces.go Updates executor MetricLabeler to embed ccvcommon.CurseCheckerMetrics and removes inlined curse methods.
Comments suppressed due to low confidence (1)

verifier/pkg/sourcereader/finality_checker.go:69

  • NewFinalityViolationCheckerService accepts metrics but doesn't validate it. If a nil implementation is passed, later calls to SetVerifierFinalityViolated (including in reset() and on violation detection) will panic. Consider either rejecting nil metrics in the constructor (consistent with other deps) or defaulting to a no-op implementation when nil is provided.
func NewFinalityViolationCheckerService(
	sourceReader chainaccess.SourceReader,
	chainSelector protocol.ChainSelector,
	lggr logger.Logger,
	metrics verifier.FinalityCheckerMetrics,
) (*FinalityViolationCheckerService, error) {
	if sourceReader == nil {
		return nil, fmt.Errorf("sourceReader cannot be nil")
	}
	if lggr == nil {
		return nil, fmt.Errorf("logger cannot be nil")
	}

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

@bukata-sa bukata-sa enabled auto-merge April 21, 2026 15:37
@bukata-sa bukata-sa added this pull request to the merge queue Apr 21, 2026
Merged via the queue into main with commit 89639df Apr 21, 2026
33 of 34 checks passed
@bukata-sa bukata-sa deleted the ccip-9669/ccip_o11y_finality_violation branch April 21, 2026 15:56
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants