Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
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
22 changes: 22 additions & 0 deletions .mockery.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -121,3 +121,25 @@ packages:
configs:
- dir: ./verifier/internal/mocks
outpkg: mocks

# The Checker mock gets its own package rather than the flat verifier/internal/mocks because
# its consumer, verifier/pkg/commit, cannot import the flat one: that package holds the cctp
# attestation mock, so it depends on verifier/pkg/token/cctp, which depends on commit. A
# commit test importing it closes that loop. This directory imports only policy, so it does
# not.
#
# It cannot serve verifier/pkg/policy's own tests either: gate_test.go is package policy, and
# any mocks package importing policy is a cycle for an in-package test. Those tests use the
# hand-written stubChecker in gate_test.go and should keep doing so.
#
# The generated signature names policy/internal/policyapi, which is where the request type is
# defined. That is not a barrier for callers: policy.EvaluateRequest is a true alias for it,
# so a commit test writes Run(func(_ context.Context, req policy.EvaluateRequest){...}) and it
# type-checks. Rewriting it with mockery's replace-type does not work, because the target is
# an alias and mockery emits an empty type name for it.
github.com/smartcontractkit/chainlink-ccv/verifier/pkg/policy:
interfaces:
Checker:
configs:
- dir: ./verifier/pkg/policy/mocks
outpkg: mocks
42 changes: 20 additions & 22 deletions verifier/pkg/commit/policy_hook_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -6,11 +6,13 @@ import (
"testing"

"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/mock"
"github.com/stretchr/testify/require"

"github.com/smartcontractkit/chainlink-ccv/protocol"
"github.com/smartcontractkit/chainlink-ccv/verifier/pkg/monitoring"
"github.com/smartcontractkit/chainlink-ccv/verifier/pkg/policy"
"github.com/smartcontractkit/chainlink-ccv/verifier/pkg/policy/mocks"
verifier "github.com/smartcontractkit/chainlink-ccv/verifier/pkg/vtypes"
"github.com/smartcontractkit/chainlink-common/pkg/logger"
)
Expand Down Expand Up @@ -114,19 +116,6 @@ type failingSigner struct {

func (s failingSigner) Sign([]byte) ([]byte, error) { return nil, s.err }

// recordingChecker answers every message with the same verdict and counts the calls, so a test
// can tell "the endpoint passed it and the verifier still refused" apart from "the endpoint was
// never asked".
type recordingChecker struct {
verdict policy.Verdict
calls int
}

func (c *recordingChecker) Evaluate(context.Context, policy.EvaluateRequest) (policy.Verdict, error) {
c.calls++
return c.verdict, nil
}

// TestPolicyHook_PassCannotBypassVerification is the answer to "are the policy checks purely
// additive": they are. A PASS is not an instruction to sign. It only lets the task reach the
// commit verifier, which then applies every check it would have applied with no hook configured.
Expand Down Expand Up @@ -158,19 +147,29 @@ func TestPolicyHook_PassCannotBypassVerification(t *testing.T) {
logger.Test(t), monitoring.NewFakeVerifierMonitoring())
require.NoError(t, err)

checker := &recordingChecker{verdict: policy.Verdict{Decision: policy.DecisionPass}}
// The endpoint must have been asked exactly once and answered PASS, or this test proves
// nothing about what a PASS can do: .Once() plus the mock's cleanup assertion pins both
// halves of that.
task := newVerifiableTask(t, configuredSourceChain, destChain, addr, verifierBlob, executorAddr)

checker := mocks.NewMockChecker(t)
checker.EXPECT().Evaluate(mock.Anything, mock.Anything).
// Typed rather than untyped mock.Arguments. The generated signature names
// policy/internal/policyapi, which this package cannot import, but
// policy.EvaluateRequest is a true alias for that type, so naming it here
// type-checks. It also pins that the gate asked about the task in hand.
Run(func(_ context.Context, req policy.EvaluateRequest) {
assert.Equal(t, task.MessageID, req.MessageId)
}).
Return(policy.Verdict{Decision: policy.DecisionPass}, nil).Once()
gated, err := policy.NewGatedVerifier(
logger.Test(t), "committee-verifier-1", cv, checker,
monitoring.NewFakeVerifierMonitoring(), 0,
)
require.NoError(t, err)

task := newVerifiableTask(t, configuredSourceChain, destChain, addr, verifierBlob, executorAddr)
results := gated.VerifyMessages(t.Context(), []verifier.VerificationTask{task})

require.Equal(t, 1, checker.calls,
"the endpoint must have been asked and answered PASS, or this proves nothing about what a PASS can do")

require.Len(t, results, 1)
assert.Nil(t, results[0].Result, "a PASS must not produce a signature the verifier withheld")
require.NotNil(t, results[0].Error)
Expand All @@ -191,7 +190,9 @@ func TestPolicyHook_PassCannotBypassVerification(t *testing.T) {
cv, err := NewCommitVerifier(config, addr, signer, logger.Test(t), monitoring.NewFakeVerifierMonitoring())
require.NoError(t, err)

checker := &recordingChecker{verdict: policy.Verdict{Decision: policy.DecisionPass}}
// No expectation is set on the mock: the mock fails the test if Evaluate is called at
// all, which is how the endpoint skip stays asserted rather than assumed.
checker := mocks.NewMockChecker(t)
gated, err := policy.NewGatedVerifier(
logger.Test(t), "committee-verifier-1", cv, checker,
monitoring.NewFakeVerifierMonitoring(), 0,
Expand All @@ -201,9 +202,6 @@ func TestPolicyHook_PassCannotBypassVerification(t *testing.T) {
task := newVerifiableTask(t, unconfiguredSourceChain, destChain, addr, verifierBlob, executorAddr)
results := gated.VerifyMessages(t.Context(), []verifier.VerificationTask{task})

assert.Zero(t, checker.calls,
"the hook gates messages this verifier would sign, so a task it already rejects costs the operator nothing")

require.Len(t, results, 1)
assert.Nil(t, results[0].Result, "a task the verifier rejects is never signed")
require.NotNil(t, results[0].Error)
Expand Down
95 changes: 95 additions & 0 deletions verifier/pkg/policy/mocks/mock_Checker.go

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Loading