From 83f7413501593058443687347d0b2078410bcc1d Mon Sep 17 00:00:00 2001 From: Matthew Pendrey Date: Thu, 12 Dec 2024 16:57:18 +0000 Subject: [PATCH] Refactor workflow registry to use querykeys api (#15638) * temp disable test * wip * fix duplicate event bug - change contract reader poll query from Gte to Gt * event state sync test added and passing with fixes - single event * added methods to create different event types * pre-refactor checkpoint - up to this commit no changes have been made to wf registry - test has been added to confirm (and actually has found bugs) in current wf registry behaviour * refactor part 1: replaced querykey with querykeys - all tests passing * refactor part 2: collapse individual event query into single events query and reduce goroutine and channel usage * refactor part 3 - down to single ticker thread * tidy * enable the event ordering test that was failing in the old workflow syncer * lint remove change to try and resolve flaky eth smoke tests * log fix --- .mockery.yaml | 6 - .../workflows/syncer/workflow_syncer_test.go | 215 +++++++++++- .../workflows/syncer/contract_reader_mock.go | 302 ----------------- core/services/workflows/syncer/heap.go | 63 ---- .../workflows/syncer/workflow_registry.go | 309 +++++------------- .../syncer/workflow_registry_test.go | 234 ------------- 6 files changed, 291 insertions(+), 838 deletions(-) delete mode 100644 core/services/workflows/syncer/contract_reader_mock.go delete mode 100644 core/services/workflows/syncer/heap.go delete mode 100644 core/services/workflows/syncer/workflow_registry_test.go diff --git a/.mockery.yaml b/.mockery.yaml index dd9024cc066..5777ca1da92 100644 --- a/.mockery.yaml +++ b/.mockery.yaml @@ -583,12 +583,6 @@ packages: github.com/smartcontractkit/chainlink/v2/core/services/workflows/syncer: interfaces: ORM: - ContractReader: - config: - mockname: "Mock{{ .InterfaceName }}" - filename: contract_reader_mock.go - inpackage: true - dir: "{{ .InterfaceDir }}" Handler: config: mockname: "Mock{{ .InterfaceName }}" diff --git a/core/services/relay/evm/capabilities/workflows/syncer/workflow_syncer_test.go b/core/services/relay/evm/capabilities/workflows/syncer/workflow_syncer_test.go index 3c6ee8a1d04..066e85e839f 100644 --- a/core/services/relay/evm/capabilities/workflows/syncer/workflow_syncer_test.go +++ b/core/services/relay/evm/capabilities/workflows/syncer/workflow_syncer_test.go @@ -6,7 +6,9 @@ import ( "encoding/base64" "encoding/hex" "fmt" + rand2 "math/rand/v2" "strings" + "sync" "testing" "time" @@ -31,17 +33,38 @@ import ( "github.com/smartcontractkit/chainlink/v2/core/utils/crypto" "github.com/stretchr/testify/require" + + crypto2 "github.com/ethereum/go-ethereum/crypto" ) type testEvtHandler struct { events []syncer.Event + mux sync.Mutex } func (m *testEvtHandler) Handle(ctx context.Context, event syncer.Event) error { + m.mux.Lock() + defer m.mux.Unlock() m.events = append(m.events, event) return nil } +func (m *testEvtHandler) ClearEvents() { + m.mux.Lock() + defer m.mux.Unlock() + m.events = make([]syncer.Event, 0) +} + +func (m *testEvtHandler) GetEvents() []syncer.Event { + m.mux.Lock() + defer m.mux.Unlock() + + eventsCopy := make([]syncer.Event, len(m.events)) + copy(eventsCopy, m.events) + + return eventsCopy +} + func newTestEvtHandler() *testEvtHandler { return &testEvtHandler{ events: make([]syncer.Event, 0), @@ -68,6 +91,138 @@ func (m *testWorkflowRegistryContractLoader) LoadWorkflows(ctx context.Context, }, nil } +func Test_EventHandlerStateSync(t *testing.T) { + lggr := logger.TestLogger(t) + backendTH := testutils.NewEVMBackendTH(t) + donID := uint32(1) + + eventPollTicker := time.NewTicker(50 * time.Millisecond) + defer eventPollTicker.Stop() + + // Deploy a test workflow_registry + wfRegistryAddr, _, wfRegistryC, err := workflow_registry_wrapper.DeployWorkflowRegistry(backendTH.ContractsOwner, backendTH.Backend.Client()) + backendTH.Backend.Commit() + require.NoError(t, err) + + // setup contract state to allow the secrets to be updated + updateAllowedDONs(t, backendTH, wfRegistryC, []uint32{donID}, true) + updateAuthorizedAddress(t, backendTH, wfRegistryC, []common.Address{backendTH.ContractsOwner.From}, true) + + // Create some initial static state + numberWorkflows := 20 + for i := 0; i < numberWorkflows; i++ { + var workflowID [32]byte + _, err = rand.Read((workflowID)[:]) + require.NoError(t, err) + workflow := RegisterWorkflowCMD{ + Name: fmt.Sprintf("test-wf-%d", i), + DonID: donID, + Status: uint8(1), + SecretsURL: "someurl", + } + workflow.ID = workflowID + registerWorkflow(t, backendTH, wfRegistryC, workflow) + } + + testEventHandler := newTestEvtHandler() + loader := syncer.NewWorkflowRegistryContractLoader(lggr, wfRegistryAddr.Hex(), func(ctx context.Context, bytes []byte) (syncer.ContractReader, error) { + return backendTH.NewContractReader(ctx, t, bytes) + }, testEventHandler) + + // Create the registry + registry := syncer.NewWorkflowRegistry( + lggr, + func(ctx context.Context, bytes []byte) (syncer.ContractReader, error) { + return backendTH.NewContractReader(ctx, t, bytes) + }, + wfRegistryAddr.Hex(), + syncer.WorkflowEventPollerConfig{ + QueryCount: 20, + }, + testEventHandler, + loader, + &testDonNotifier{ + don: capabilities.DON{ + ID: donID, + }, + err: nil, + }, + syncer.WithTicker(eventPollTicker.C), + ) + + servicetest.Run(t, registry) + + require.Eventually(t, func() bool { + numEvents := len(testEventHandler.GetEvents()) + return numEvents == numberWorkflows + }, 5*time.Second, time.Second) + + for _, event := range testEventHandler.GetEvents() { + assert.Equal(t, syncer.WorkflowRegisteredEvent, event.GetEventType()) + } + + testEventHandler.ClearEvents() + + // Create different event types for a number of workflows and confirm that the event handler processes them in order + numberOfEventCycles := 50 + for i := 0; i < numberOfEventCycles; i++ { + var workflowID [32]byte + _, err = rand.Read((workflowID)[:]) + require.NoError(t, err) + workflow := RegisterWorkflowCMD{ + Name: "test-wf-register-event", + DonID: donID, + Status: uint8(1), + SecretsURL: "", + } + workflow.ID = workflowID + + // Generate events of different types with some jitter + registerWorkflow(t, backendTH, wfRegistryC, workflow) + time.Sleep(time.Millisecond * time.Duration(rand2.IntN(10))) + data := append(backendTH.ContractsOwner.From.Bytes(), []byte(workflow.Name)...) + workflowKey := crypto2.Keccak256Hash(data) + activateWorkflow(t, backendTH, wfRegistryC, workflowKey) + time.Sleep(time.Millisecond * time.Duration(rand2.IntN(10))) + pauseWorkflow(t, backendTH, wfRegistryC, workflowKey) + time.Sleep(time.Millisecond * time.Duration(rand2.IntN(10))) + var newWorkflowID [32]byte + _, err = rand.Read((newWorkflowID)[:]) + require.NoError(t, err) + updateWorkflow(t, backendTH, wfRegistryC, workflowKey, newWorkflowID, workflow.BinaryURL+"2", workflow.ConfigURL, workflow.SecretsURL) + time.Sleep(time.Millisecond * time.Duration(rand2.IntN(10))) + deleteWorkflow(t, backendTH, wfRegistryC, workflowKey) + } + + // Confirm the expected number of events are received in the correct order + require.Eventually(t, func() bool { + events := testEventHandler.GetEvents() + numEvents := len(events) + expectedNumEvents := 5 * numberOfEventCycles + + if numEvents == expectedNumEvents { + // verify the events are the expected types in the expected order + for idx, event := range events { + switch idx % 5 { + case 0: + assert.Equal(t, syncer.WorkflowRegisteredEvent, event.GetEventType()) + case 1: + assert.Equal(t, syncer.WorkflowActivatedEvent, event.GetEventType()) + case 2: + assert.Equal(t, syncer.WorkflowPausedEvent, event.GetEventType()) + case 3: + assert.Equal(t, syncer.WorkflowUpdatedEvent, event.GetEventType()) + case 4: + assert.Equal(t, syncer.WorkflowDeletedEvent, event.GetEventType()) + } + } + return true + } + + return false + }, 50*time.Second, time.Second) +} + func Test_InitialStateSync(t *testing.T) { lggr := logger.TestLogger(t) backendTH := testutils.NewEVMBackendTH(t) @@ -128,10 +283,10 @@ func Test_InitialStateSync(t *testing.T) { servicetest.Run(t, worker) require.Eventually(t, func() bool { - return len(testEventHandler.events) == numberWorkflows + return len(testEventHandler.GetEvents()) == numberWorkflows }, 5*time.Second, time.Second) - for _, event := range testEventHandler.events { + for _, event := range testEventHandler.GetEvents() { assert.Equal(t, syncer.WorkflowRegisteredEvent, event.GetEventType()) } } @@ -497,3 +652,59 @@ func requestForceUpdateSecrets( th.Backend.Commit() th.Backend.Commit() } + +func activateWorkflow( + t *testing.T, + th *testutils.EVMBackendTH, + wfRegC *workflow_registry_wrapper.WorkflowRegistry, + workflowKey [32]byte, +) { + t.Helper() + _, err := wfRegC.ActivateWorkflow(th.ContractsOwner, workflowKey) + require.NoError(t, err, "failed to activate workflow") + th.Backend.Commit() + th.Backend.Commit() + th.Backend.Commit() +} + +func pauseWorkflow( + t *testing.T, + th *testutils.EVMBackendTH, + wfRegC *workflow_registry_wrapper.WorkflowRegistry, + workflowKey [32]byte, +) { + t.Helper() + _, err := wfRegC.PauseWorkflow(th.ContractsOwner, workflowKey) + require.NoError(t, err, "failed to pause workflow") + th.Backend.Commit() + th.Backend.Commit() + th.Backend.Commit() +} + +func deleteWorkflow( + t *testing.T, + th *testutils.EVMBackendTH, + wfRegC *workflow_registry_wrapper.WorkflowRegistry, + workflowKey [32]byte, +) { + t.Helper() + _, err := wfRegC.DeleteWorkflow(th.ContractsOwner, workflowKey) + require.NoError(t, err, "failed to delete workflow") + th.Backend.Commit() + th.Backend.Commit() + th.Backend.Commit() +} + +func updateWorkflow( + t *testing.T, + th *testutils.EVMBackendTH, + wfRegC *workflow_registry_wrapper.WorkflowRegistry, + workflowKey [32]byte, newWorkflowID [32]byte, binaryURL string, configURL string, secretsURL string, +) { + t.Helper() + _, err := wfRegC.UpdateWorkflow(th.ContractsOwner, workflowKey, newWorkflowID, binaryURL, configURL, secretsURL) + require.NoError(t, err, "failed to update workflow") + th.Backend.Commit() + th.Backend.Commit() + th.Backend.Commit() +} diff --git a/core/services/workflows/syncer/contract_reader_mock.go b/core/services/workflows/syncer/contract_reader_mock.go deleted file mode 100644 index e6e7c8385f5..00000000000 --- a/core/services/workflows/syncer/contract_reader_mock.go +++ /dev/null @@ -1,302 +0,0 @@ -// Code generated by mockery v2.46.3. DO NOT EDIT. - -package syncer - -import ( - context "context" - - query "github.com/smartcontractkit/chainlink-common/pkg/types/query" - primitives "github.com/smartcontractkit/chainlink-common/pkg/types/query/primitives" - mock "github.com/stretchr/testify/mock" - - types "github.com/smartcontractkit/chainlink-common/pkg/types" -) - -// MockContractReader is an autogenerated mock type for the ContractReader type -type MockContractReader struct { - mock.Mock -} - -type MockContractReader_Expecter struct { - mock *mock.Mock -} - -func (_m *MockContractReader) EXPECT() *MockContractReader_Expecter { - return &MockContractReader_Expecter{mock: &_m.Mock} -} - -// Bind provides a mock function with given fields: _a0, _a1 -func (_m *MockContractReader) Bind(_a0 context.Context, _a1 []types.BoundContract) error { - ret := _m.Called(_a0, _a1) - - if len(ret) == 0 { - panic("no return value specified for Bind") - } - - var r0 error - if rf, ok := ret.Get(0).(func(context.Context, []types.BoundContract) error); ok { - r0 = rf(_a0, _a1) - } else { - r0 = ret.Error(0) - } - - return r0 -} - -// MockContractReader_Bind_Call is a *mock.Call that shadows Run/Return methods with type explicit version for method 'Bind' -type MockContractReader_Bind_Call struct { - *mock.Call -} - -// Bind is a helper method to define mock.On call -// - _a0 context.Context -// - _a1 []types.BoundContract -func (_e *MockContractReader_Expecter) Bind(_a0 interface{}, _a1 interface{}) *MockContractReader_Bind_Call { - return &MockContractReader_Bind_Call{Call: _e.mock.On("Bind", _a0, _a1)} -} - -func (_c *MockContractReader_Bind_Call) Run(run func(_a0 context.Context, _a1 []types.BoundContract)) *MockContractReader_Bind_Call { - _c.Call.Run(func(args mock.Arguments) { - run(args[0].(context.Context), args[1].([]types.BoundContract)) - }) - return _c -} - -func (_c *MockContractReader_Bind_Call) Return(_a0 error) *MockContractReader_Bind_Call { - _c.Call.Return(_a0) - return _c -} - -func (_c *MockContractReader_Bind_Call) RunAndReturn(run func(context.Context, []types.BoundContract) error) *MockContractReader_Bind_Call { - _c.Call.Return(run) - return _c -} - -// Close provides a mock function with given fields: -func (_m *MockContractReader) Close() error { - ret := _m.Called() - - if len(ret) == 0 { - panic("no return value specified for Close") - } - - var r0 error - if rf, ok := ret.Get(0).(func() error); ok { - r0 = rf() - } else { - r0 = ret.Error(0) - } - - return r0 -} - -// MockContractReader_Close_Call is a *mock.Call that shadows Run/Return methods with type explicit version for method 'Close' -type MockContractReader_Close_Call struct { - *mock.Call -} - -// Close is a helper method to define mock.On call -func (_e *MockContractReader_Expecter) Close() *MockContractReader_Close_Call { - return &MockContractReader_Close_Call{Call: _e.mock.On("Close")} -} - -func (_c *MockContractReader_Close_Call) Run(run func()) *MockContractReader_Close_Call { - _c.Call.Run(func(args mock.Arguments) { - run() - }) - return _c -} - -func (_c *MockContractReader_Close_Call) Return(_a0 error) *MockContractReader_Close_Call { - _c.Call.Return(_a0) - return _c -} - -func (_c *MockContractReader_Close_Call) RunAndReturn(run func() error) *MockContractReader_Close_Call { - _c.Call.Return(run) - return _c -} - -// GetLatestValueWithHeadData provides a mock function with given fields: ctx, readName, confidenceLevel, params, returnVal -func (_m *MockContractReader) GetLatestValueWithHeadData(ctx context.Context, readName string, confidenceLevel primitives.ConfidenceLevel, params any, returnVal any) (*types.Head, error) { - ret := _m.Called(ctx, readName, confidenceLevel, params, returnVal) - - if len(ret) == 0 { - panic("no return value specified for GetLatestValueWithHeadData") - } - - var r0 *types.Head - var r1 error - if rf, ok := ret.Get(0).(func(context.Context, string, primitives.ConfidenceLevel, any, any) (*types.Head, error)); ok { - return rf(ctx, readName, confidenceLevel, params, returnVal) - } - if rf, ok := ret.Get(0).(func(context.Context, string, primitives.ConfidenceLevel, any, any) *types.Head); ok { - r0 = rf(ctx, readName, confidenceLevel, params, returnVal) - } else { - if ret.Get(0) != nil { - r0 = ret.Get(0).(*types.Head) - } - } - - if rf, ok := ret.Get(1).(func(context.Context, string, primitives.ConfidenceLevel, any, any) error); ok { - r1 = rf(ctx, readName, confidenceLevel, params, returnVal) - } else { - r1 = ret.Error(1) - } - - return r0, r1 -} - -// MockContractReader_GetLatestValueWithHeadData_Call is a *mock.Call that shadows Run/Return methods with type explicit version for method 'GetLatestValueWithHeadData' -type MockContractReader_GetLatestValueWithHeadData_Call struct { - *mock.Call -} - -// GetLatestValueWithHeadData is a helper method to define mock.On call -// - ctx context.Context -// - readName string -// - confidenceLevel primitives.ConfidenceLevel -// - params any -// - returnVal any -func (_e *MockContractReader_Expecter) GetLatestValueWithHeadData(ctx interface{}, readName interface{}, confidenceLevel interface{}, params interface{}, returnVal interface{}) *MockContractReader_GetLatestValueWithHeadData_Call { - return &MockContractReader_GetLatestValueWithHeadData_Call{Call: _e.mock.On("GetLatestValueWithHeadData", ctx, readName, confidenceLevel, params, returnVal)} -} - -func (_c *MockContractReader_GetLatestValueWithHeadData_Call) Run(run func(ctx context.Context, readName string, confidenceLevel primitives.ConfidenceLevel, params any, returnVal any)) *MockContractReader_GetLatestValueWithHeadData_Call { - _c.Call.Run(func(args mock.Arguments) { - run(args[0].(context.Context), args[1].(string), args[2].(primitives.ConfidenceLevel), args[3].(any), args[4].(any)) - }) - return _c -} - -func (_c *MockContractReader_GetLatestValueWithHeadData_Call) Return(head *types.Head, err error) *MockContractReader_GetLatestValueWithHeadData_Call { - _c.Call.Return(head, err) - return _c -} - -func (_c *MockContractReader_GetLatestValueWithHeadData_Call) RunAndReturn(run func(context.Context, string, primitives.ConfidenceLevel, any, any) (*types.Head, error)) *MockContractReader_GetLatestValueWithHeadData_Call { - _c.Call.Return(run) - return _c -} - -// QueryKey provides a mock function with given fields: _a0, _a1, _a2, _a3, _a4 -func (_m *MockContractReader) QueryKey(_a0 context.Context, _a1 types.BoundContract, _a2 query.KeyFilter, _a3 query.LimitAndSort, _a4 any) ([]types.Sequence, error) { - ret := _m.Called(_a0, _a1, _a2, _a3, _a4) - - if len(ret) == 0 { - panic("no return value specified for QueryKey") - } - - var r0 []types.Sequence - var r1 error - if rf, ok := ret.Get(0).(func(context.Context, types.BoundContract, query.KeyFilter, query.LimitAndSort, any) ([]types.Sequence, error)); ok { - return rf(_a0, _a1, _a2, _a3, _a4) - } - if rf, ok := ret.Get(0).(func(context.Context, types.BoundContract, query.KeyFilter, query.LimitAndSort, any) []types.Sequence); ok { - r0 = rf(_a0, _a1, _a2, _a3, _a4) - } else { - if ret.Get(0) != nil { - r0 = ret.Get(0).([]types.Sequence) - } - } - - if rf, ok := ret.Get(1).(func(context.Context, types.BoundContract, query.KeyFilter, query.LimitAndSort, any) error); ok { - r1 = rf(_a0, _a1, _a2, _a3, _a4) - } else { - r1 = ret.Error(1) - } - - return r0, r1 -} - -// MockContractReader_QueryKey_Call is a *mock.Call that shadows Run/Return methods with type explicit version for method 'QueryKey' -type MockContractReader_QueryKey_Call struct { - *mock.Call -} - -// QueryKey is a helper method to define mock.On call -// - _a0 context.Context -// - _a1 types.BoundContract -// - _a2 query.KeyFilter -// - _a3 query.LimitAndSort -// - _a4 any -func (_e *MockContractReader_Expecter) QueryKey(_a0 interface{}, _a1 interface{}, _a2 interface{}, _a3 interface{}, _a4 interface{}) *MockContractReader_QueryKey_Call { - return &MockContractReader_QueryKey_Call{Call: _e.mock.On("QueryKey", _a0, _a1, _a2, _a3, _a4)} -} - -func (_c *MockContractReader_QueryKey_Call) Run(run func(_a0 context.Context, _a1 types.BoundContract, _a2 query.KeyFilter, _a3 query.LimitAndSort, _a4 any)) *MockContractReader_QueryKey_Call { - _c.Call.Run(func(args mock.Arguments) { - run(args[0].(context.Context), args[1].(types.BoundContract), args[2].(query.KeyFilter), args[3].(query.LimitAndSort), args[4].(any)) - }) - return _c -} - -func (_c *MockContractReader_QueryKey_Call) Return(_a0 []types.Sequence, _a1 error) *MockContractReader_QueryKey_Call { - _c.Call.Return(_a0, _a1) - return _c -} - -func (_c *MockContractReader_QueryKey_Call) RunAndReturn(run func(context.Context, types.BoundContract, query.KeyFilter, query.LimitAndSort, any) ([]types.Sequence, error)) *MockContractReader_QueryKey_Call { - _c.Call.Return(run) - return _c -} - -// Start provides a mock function with given fields: ctx -func (_m *MockContractReader) Start(ctx context.Context) error { - ret := _m.Called(ctx) - - if len(ret) == 0 { - panic("no return value specified for Start") - } - - var r0 error - if rf, ok := ret.Get(0).(func(context.Context) error); ok { - r0 = rf(ctx) - } else { - r0 = ret.Error(0) - } - - return r0 -} - -// MockContractReader_Start_Call is a *mock.Call that shadows Run/Return methods with type explicit version for method 'Start' -type MockContractReader_Start_Call struct { - *mock.Call -} - -// Start is a helper method to define mock.On call -// - ctx context.Context -func (_e *MockContractReader_Expecter) Start(ctx interface{}) *MockContractReader_Start_Call { - return &MockContractReader_Start_Call{Call: _e.mock.On("Start", ctx)} -} - -func (_c *MockContractReader_Start_Call) Run(run func(ctx context.Context)) *MockContractReader_Start_Call { - _c.Call.Run(func(args mock.Arguments) { - run(args[0].(context.Context)) - }) - return _c -} - -func (_c *MockContractReader_Start_Call) Return(_a0 error) *MockContractReader_Start_Call { - _c.Call.Return(_a0) - return _c -} - -func (_c *MockContractReader_Start_Call) RunAndReturn(run func(context.Context) error) *MockContractReader_Start_Call { - _c.Call.Return(run) - return _c -} - -// NewMockContractReader creates a new instance of MockContractReader. 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 NewMockContractReader(t interface { - mock.TestingT - Cleanup(func()) -}) *MockContractReader { - mock := &MockContractReader{} - mock.Mock.Test(t) - - t.Cleanup(func() { mock.AssertExpectations(t) }) - - return mock -} diff --git a/core/services/workflows/syncer/heap.go b/core/services/workflows/syncer/heap.go deleted file mode 100644 index 061293928a3..00000000000 --- a/core/services/workflows/syncer/heap.go +++ /dev/null @@ -1,63 +0,0 @@ -package syncer - -import "container/heap" - -type Heap interface { - // Push adds a new item to the heap. - Push(x WorkflowRegistryEventResponse) - - // Pop removes the smallest item from the heap and returns it. - Pop() WorkflowRegistryEventResponse - - // Len returns the number of items in the heap. - Len() int -} - -// publicHeap is a wrapper around the heap.Interface that exposes the Push and Pop methods. -type publicHeap[T any] struct { - heap heap.Interface -} - -func (h *publicHeap[T]) Push(x T) { - heap.Push(h.heap, x) -} - -func (h *publicHeap[T]) Pop() T { - return heap.Pop(h.heap).(T) -} - -func (h *publicHeap[T]) Len() int { - return h.heap.Len() -} - -// blockHeightHeap is a heap.Interface that sorts WorkflowRegistryEventResponses by block height. -type blockHeightHeap []WorkflowRegistryEventResponse - -// newBlockHeightHeap returns an initialized heap that sorts WorkflowRegistryEventResponses by block height. -func newBlockHeightHeap() Heap { - h := blockHeightHeap(make([]WorkflowRegistryEventResponse, 0)) - heap.Init(&h) - return &publicHeap[WorkflowRegistryEventResponse]{heap: &h} -} - -func (h *blockHeightHeap) Len() int { return len(*h) } - -func (h *blockHeightHeap) Less(i, j int) bool { - return (*h)[i].Event.Head.Height < (*h)[j].Event.Head.Height -} - -func (h *blockHeightHeap) Swap(i, j int) { - (*h)[i], (*h)[j] = (*h)[j], (*h)[i] -} - -func (h *blockHeightHeap) Push(x any) { - *h = append(*h, x.(WorkflowRegistryEventResponse)) -} - -func (h *blockHeightHeap) Pop() any { - old := *h - n := len(old) - x := old[n-1] - *h = old[0 : n-1] - return x -} diff --git a/core/services/workflows/syncer/workflow_registry.go b/core/services/workflows/syncer/workflow_registry.go index 75fcc9735ad..223fbe8e758 100644 --- a/core/services/workflows/syncer/workflow_registry.go +++ b/core/services/workflows/syncer/workflow_registry.go @@ -5,13 +5,14 @@ import ( "encoding/hex" "encoding/json" "fmt" + "iter" "sync" "time" "github.com/smartcontractkit/chainlink-common/pkg/capabilities" "github.com/smartcontractkit/chainlink-common/pkg/services" - types "github.com/smartcontractkit/chainlink-common/pkg/types" - query "github.com/smartcontractkit/chainlink-common/pkg/types/query" + "github.com/smartcontractkit/chainlink-common/pkg/types" + "github.com/smartcontractkit/chainlink-common/pkg/types/query" "github.com/smartcontractkit/chainlink-common/pkg/types/query/primitives" "github.com/smartcontractkit/chainlink-common/pkg/values" "github.com/smartcontractkit/chainlink/v2/core/gethwrappers/workflow/generated/workflow_registry_wrapper" @@ -90,19 +91,19 @@ type WorkflowLoadConfig struct { // FetcherFunc is an abstraction for fetching the contents stored at a URL. type FetcherFunc func(ctx context.Context, url string) ([]byte, error) -type ContractReaderFactory interface { - NewContractReader(context.Context, []byte) (types.ContractReader, error) -} - // ContractReader is a subset of types.ContractReader defined locally to enable mocking. type ContractReader interface { Start(ctx context.Context) error Close() error Bind(context.Context, []types.BoundContract) error - QueryKey(context.Context, types.BoundContract, query.KeyFilter, query.LimitAndSort, any) ([]types.Sequence, error) + QueryKeys(ctx context.Context, keyQueries []types.ContractKeyFilter, limitAndSort query.LimitAndSort) (iter.Seq2[string, types.Sequence], error) GetLatestValueWithHeadData(ctx context.Context, readName string, confidenceLevel primitives.ConfidenceLevel, params any, returnVal any) (head *types.Head, err error) } +type ContractReaderFactory interface { + NewContractReader(context.Context, []byte) (types.ContractReader, error) +} + // WorkflowRegistrySyncer is the public interface of the package. type WorkflowRegistrySyncer interface { services.Service @@ -128,21 +129,11 @@ type workflowRegistry struct { newContractReaderFn newContractReaderFn - eventPollerCfg WorkflowEventPollerConfig - eventTypes []WorkflowRegistryEventType - - // eventsCh is read by the handler and each event is handled once received. - eventsCh chan WorkflowRegistryEventResponse + eventPollerCfg WorkflowEventPollerConfig + eventTypes []WorkflowRegistryEventType handler evtHandler initialWorkflowsStateLoader initialWorkflowsStateLoader - // batchCh is a channel that receives batches of events from the contract query goroutines. - batchCh chan []WorkflowRegistryEventResponse - - // heap is a min heap that merges batches of events from the contract query goroutines. The - // default min heap is sorted by block height. - heap Heap - workflowDonNotifier donNotifier reader ContractReader @@ -197,11 +188,8 @@ func NewWorkflowRegistry( newContractReaderFn: newContractReaderFn, workflowRegistryAddress: addr, eventPollerCfg: eventPollerConfig, - heap: newBlockHeightHeap(), stopCh: make(services.StopChan), eventTypes: ets, - eventsCh: make(chan WorkflowRegistryEventResponse), - batchCh: make(chan []WorkflowRegistryEventResponse, len(ets)), handler: handler, initialWorkflowsStateLoader: initialWorkflowsStateLoader, workflowDonNotifier: workflowDonNotifier, @@ -238,15 +226,13 @@ func (w *workflowRegistry) Start(_ context.Context) error { return } - w.syncEventsLoop(ctx, loadWorkflowsHead.Height) - }() - - w.wg.Add(1) - go func() { - defer w.wg.Done() - defer cancel() + reader, err := w.getContractReader(ctx) + if err != nil { + w.lggr.Criticalf("contract reader unavailable : %s", err) + return + } - w.handlerLoop(ctx) + w.readRegistryEvents(ctx, reader, loadWorkflowsHead.Height) }() return nil @@ -273,135 +259,82 @@ func (w *workflowRegistry) Name() string { return name } -// handlerLoop handles the events that are emitted by the contract. -func (w *workflowRegistry) handlerLoop(ctx context.Context) { +// readRegistryEvents polls the contract for events and send them to the events channel. +func (w *workflowRegistry) readRegistryEvents(ctx context.Context, reader ContractReader, lastReadBlockNumber string) { + ticker := w.getTicker() + + var keyQueries = make([]types.ContractKeyFilter, 0, len(w.eventTypes)) + for _, et := range w.eventTypes { + var logData values.Value + keyQueries = append(keyQueries, types.ContractKeyFilter{ + KeyFilter: query.KeyFilter{ + Key: string(et), + Expressions: []query.Expression{ + query.Confidence(primitives.Finalized), + query.Block(lastReadBlockNumber, primitives.Gt), + }, + }, + Contract: types.BoundContract{ + Name: WorkflowRegistryContractName, + Address: w.workflowRegistryAddress, + }, + SequenceDataType: &logData, + }) + } + + cursor := "" for { select { case <-ctx.Done(): return - case resp, open := <-w.eventsCh: - if !open { - return + case <-ticker: + limitAndSort := query.LimitAndSort{ + SortBy: []query.SortBy{query.NewSortByTimestamp(query.Asc)}, + Limit: query.Limit{Count: w.eventPollerCfg.QueryCount}, } - - if resp.Err != nil || resp.Event == nil { - w.lggr.Errorw("failed to handle event", "err", resp.Err) - continue + if cursor != "" { + limitAndSort.Limit = query.CursorLimit(cursor, query.CursorFollowing, w.eventPollerCfg.QueryCount) } - event := resp.Event - w.lggr.Debugf("handling event: %+v", event) - if err := w.handler.Handle(ctx, *event); err != nil { - w.lggr.Errorw("failed to handle event", "event", event, "err", err) + logsIter, err := reader.QueryKeys(ctx, keyQueries, limitAndSort) + if err != nil { + w.lggr.Errorw("failed to query keys", "err", err) continue } - } - } -} -// syncEventsLoop polls the contract for events and passes them to a channel for handling. -func (w *workflowRegistry) syncEventsLoop(ctx context.Context, lastReadBlockNumber string) { - var ( - // sendLog is a helper that sends a WorkflowRegistryEventResponse to the eventsCh in a - // blocking way that will send the response or be canceled. - sendLog = func(resp WorkflowRegistryEventResponse) { - select { - case w.eventsCh <- resp: - case <-ctx.Done(): + var logs []sequenceWithEventType + for eventType, log := range logsIter { + logs = append(logs, sequenceWithEventType{ + Sequence: log, + EventType: WorkflowRegistryEventType(eventType), + }) } - } - - ticker = w.getTicker() - - signals = make(map[WorkflowRegistryEventType]chan struct{}, 0) - ) - - // critical failure if there is no reader, the loop will exit and the parent context will be - // canceled. - reader, err := w.getContractReader(ctx) - if err != nil { - w.lggr.Criticalf("contract reader unavailable : %s", err) - return - } - - // fan out and query for each event type - for i := 0; i < len(w.eventTypes); i++ { - signal := make(chan struct{}, 1) - signals[w.eventTypes[i]] = signal - w.wg.Add(1) - go func() { - defer w.wg.Done() - - queryEvent( - ctx, - signal, - w.lggr, - reader, - lastReadBlockNumber, - queryEventConfig{ - ContractName: WorkflowRegistryContractName, - ContractAddress: w.workflowRegistryAddress, - WorkflowEventPollerConfig: w.eventPollerCfg, - }, - w.eventTypes[i], - w.batchCh, - ) - }() - } + w.lggr.Debugw("QueryKeys called", "logs", len(logs), "eventTypes", w.eventTypes, "lastReadBlockNumber", lastReadBlockNumber, "logCursor", cursor) - // Periodically send a signal to all the queryEvent goroutines to query the contract - for { - select { - case <-ctx.Done(): - return - case <-ticker: - w.lggr.Debugw("Syncing with WorkflowRegistry") - // for each event type, send a signal for it to execute a query and produce a new - // batch of event logs - for i := 0; i < len(w.eventTypes); i++ { - signal := signals[w.eventTypes[i]] - select { - case signal <- struct{}{}: - case <-ctx.Done(): - return - } + // ChainReader QueryKey API provides logs including the cursor value and not + // after the cursor value. If the response only consists of the log corresponding + // to the cursor and no log after it, then we understand that there are no new + // logs + if len(logs) == 1 && logs[0].Sequence.Cursor == cursor { + w.lggr.Infow("No new logs since", "cursor", cursor) + continue } - // block on fan-in until all fetched event logs are sent to the handlers - w.orderAndSend( - ctx, - len(w.eventTypes), - w.batchCh, - sendLog, - ) - } - } -} + var events []WorkflowRegistryEventResponse + for _, log := range logs { + if log.Sequence.Cursor == cursor { + continue + } -// orderAndSend reads n batches from the batch channel, heapifies all the batches then dequeues -// the min heap via the sendLog function. -func (w *workflowRegistry) orderAndSend( - ctx context.Context, - batchCount int, - batchCh <-chan []WorkflowRegistryEventResponse, - sendLog func(WorkflowRegistryEventResponse), -) { - for { - select { - case <-ctx.Done(): - return - case batch := <-batchCh: - for _, response := range batch { - w.heap.Push(response) + events = append(events, toWorkflowRegistryEventResponse(log.Sequence, log.EventType, w.lggr)) + cursor = log.Sequence.Cursor } - batchCount-- - // If we have received responses for all the events, then we can drain the heap. - if batchCount == 0 { - for w.heap.Len() > 0 { - sendLog(w.heap.Pop()) + for _, event := range events { + err := w.handler.Handle(ctx, event.Event) + if err != nil { + w.lggr.Errorw("failed to handle event", "err", err) } - return } } } @@ -437,95 +370,9 @@ func (w *workflowRegistry) getContractReader(ctx context.Context) (ContractReade return w.reader, nil } -type queryEventConfig struct { - ContractName string - ContractAddress string - WorkflowEventPollerConfig -} - -// queryEvent queries the contract for events of the given type on each tick from the ticker. -// Sends a batch of event logs to the batch channel. The batch represents all the -// event logs read since the last query. Loops until the context is canceled. -func queryEvent( - ctx context.Context, - ticker <-chan struct{}, - lggr logger.Logger, - reader ContractReader, - lastReadBlockNumber string, - cfg queryEventConfig, - et WorkflowRegistryEventType, - batchCh chan<- []WorkflowRegistryEventResponse, -) { - // create query - var ( - logData values.Value - cursor = "" - limitAndSort = query.LimitAndSort{ - SortBy: []query.SortBy{query.NewSortByTimestamp(query.Asc)}, - Limit: query.Limit{Count: cfg.QueryCount}, - } - bc = types.BoundContract{ - Name: cfg.ContractName, - Address: cfg.ContractAddress, - } - ) - - // Loop until canceled - for { - select { - case <-ctx.Done(): - return - case <-ticker: - responseBatch := []WorkflowRegistryEventResponse{} - - if cursor != "" { - limitAndSort.Limit = query.CursorLimit(cursor, query.CursorFollowing, cfg.QueryCount) - } - - logs, err := reader.QueryKey( - ctx, - bc, - query.KeyFilter{ - Key: string(et), - Expressions: []query.Expression{ - query.Confidence(primitives.Finalized), - query.Block(lastReadBlockNumber, primitives.Gte), - }, - }, - limitAndSort, - &logData, - ) - lcursor := cursor - if lcursor == "" { - lcursor = "empty" - } - lggr.Debugw("QueryKeys called", "logs", len(logs), "eventType", et, "lastReadBlockNumber", lastReadBlockNumber, "logCursor", lcursor) - - if err != nil { - lggr.Errorw("QueryKey failure", "err", err) - continue - } - - // ChainReader QueryKey API provides logs including the cursor value and not - // after the cursor value. If the response only consists of the log corresponding - // to the cursor and no log after it, then we understand that there are no new - // logs - if len(logs) == 1 && logs[0].Cursor == cursor { - lggr.Infow("No new logs since", "cursor", cursor) - continue - } - - for _, log := range logs { - if log.Cursor == cursor { - continue - } - - responseBatch = append(responseBatch, toWorkflowRegistryEventResponse(log, et, lggr)) - cursor = log.Cursor - } - batchCh <- responseBatch - } - } +type sequenceWithEventType struct { + Sequence types.Sequence + EventType WorkflowRegistryEventType } func getWorkflowRegistryEventReader( @@ -681,7 +528,7 @@ func (l *workflowRegistryContractLoader) LoadWorkflows(ctx context.Context, don var workflows GetWorkflowMetadataListByDONReturnVal headAtLastRead, err = contractReader.GetLatestValueWithHeadData(ctx, readIdentifier, primitives.Finalized, params, &workflows) if err != nil { - return nil, fmt.Errorf("failed to get workflow metadata for don %w", err) + return nil, fmt.Errorf("failed to get lastest value with head data %w", err) } l.lggr.Debugw("Rehydrating existing workflows", "len", len(workflows.WorkflowMetadataList)) diff --git a/core/services/workflows/syncer/workflow_registry_test.go b/core/services/workflows/syncer/workflow_registry_test.go deleted file mode 100644 index 621d3d123d5..00000000000 --- a/core/services/workflows/syncer/workflow_registry_test.go +++ /dev/null @@ -1,234 +0,0 @@ -package syncer - -import ( - "context" - "encoding/hex" - "testing" - "time" - - "github.com/stretchr/testify/mock" - - "github.com/jonboulle/clockwork" - - "github.com/smartcontractkit/chainlink-common/pkg/capabilities" - "github.com/smartcontractkit/chainlink-common/pkg/custmsg" - "github.com/smartcontractkit/chainlink-common/pkg/services/servicetest" - types "github.com/smartcontractkit/chainlink-common/pkg/types" - query "github.com/smartcontractkit/chainlink-common/pkg/types/query" - "github.com/smartcontractkit/chainlink-common/pkg/types/query/primitives" - "github.com/smartcontractkit/chainlink-common/pkg/values" - "github.com/smartcontractkit/chainlink/v2/core/internal/testutils" - "github.com/smartcontractkit/chainlink/v2/core/internal/testutils/pgtest" - "github.com/smartcontractkit/chainlink/v2/core/logger" - "github.com/smartcontractkit/chainlink/v2/core/services/keystore/keys/workflowkey" - "github.com/smartcontractkit/chainlink/v2/core/utils/crypto" - "github.com/smartcontractkit/chainlink/v2/core/utils/matches" - - "github.com/stretchr/testify/require" -) - -type testDonNotifier struct { - don capabilities.DON - err error -} - -func (t *testDonNotifier) WaitForDon(ctx context.Context) (capabilities.DON, error) { - return t.don, t.err -} - -func Test_Workflow_Registry_Syncer(t *testing.T) { - var ( - giveContents = "contents" - wantContents = "updated contents" - contractAddress = "0xdeadbeef" - giveCfg = WorkflowEventPollerConfig{ - QueryCount: 20, - } - giveURL = "http://example.com" - giveHash, err = crypto.Keccak256([]byte(giveURL)) - - giveLog = types.Sequence{ - Data: map[string]any{ - "SecretsURLHash": giveHash, - "Owner": "0xowneraddr", - }, - Cursor: "cursor", - } - ) - - require.NoError(t, err) - - var ( - lggr = logger.TestLogger(t) - db = pgtest.NewSqlxDB(t) - orm = &orm{ds: db, lggr: lggr} - ctx, cancel = context.WithCancel(testutils.Context(t)) - reader = NewMockContractReader(t) - emitter = custmsg.NewLabeler() - gateway = func(_ context.Context, _ string) ([]byte, error) { - return []byte(wantContents), nil - } - ticker = make(chan time.Time) - - handler = NewEventHandler(lggr, orm, gateway, nil, nil, - emitter, clockwork.NewFakeClock(), workflowkey.Key{}) - loader = NewWorkflowRegistryContractLoader(lggr, contractAddress, func(ctx context.Context, bytes []byte) (ContractReader, error) { - return reader, nil - }, handler) - - worker = NewWorkflowRegistry(lggr, func(ctx context.Context, bytes []byte) (ContractReader, error) { - return reader, nil - }, contractAddress, - WorkflowEventPollerConfig{ - QueryCount: 20, - }, handler, loader, - &testDonNotifier{ - don: capabilities.DON{ - ID: 1, - }, - err: nil, - }, - WithTicker(ticker)) - ) - - // Cleanup the worker - defer cancel() - - // Seed the DB with an original entry - _, err = orm.Create(ctx, giveURL, hex.EncodeToString(giveHash), giveContents) - require.NoError(t, err) - - // Mock out the contract reader query - reader.EXPECT().QueryKey( - matches.AnyContext, - types.BoundContract{ - Name: WorkflowRegistryContractName, - Address: contractAddress, - }, - query.KeyFilter{ - Key: string(ForceUpdateSecretsEvent), - Expressions: []query.Expression{ - query.Confidence(primitives.Finalized), - query.Block("0", primitives.Gte), - }, - }, - query.LimitAndSort{ - SortBy: []query.SortBy{query.NewSortByTimestamp(query.Asc)}, - Limit: query.Limit{Count: giveCfg.QueryCount}, - }, - new(values.Value), - ).Return([]types.Sequence{giveLog}, nil) - reader.EXPECT().QueryKey( - matches.AnyContext, - types.BoundContract{ - Name: WorkflowRegistryContractName, - Address: contractAddress, - }, - query.KeyFilter{ - Key: string(WorkflowPausedEvent), - Expressions: []query.Expression{ - query.Confidence(primitives.Finalized), - query.Block("0", primitives.Gte), - }, - }, - query.LimitAndSort{ - SortBy: []query.SortBy{query.NewSortByTimestamp(query.Asc)}, - Limit: query.Limit{Count: giveCfg.QueryCount}, - }, - new(values.Value), - ).Return([]types.Sequence{}, nil) - reader.EXPECT().QueryKey( - matches.AnyContext, - types.BoundContract{ - Name: WorkflowRegistryContractName, - Address: contractAddress, - }, - query.KeyFilter{ - Key: string(WorkflowDeletedEvent), - Expressions: []query.Expression{ - query.Confidence(primitives.Finalized), - query.Block("0", primitives.Gte), - }, - }, - query.LimitAndSort{ - SortBy: []query.SortBy{query.NewSortByTimestamp(query.Asc)}, - Limit: query.Limit{Count: giveCfg.QueryCount}, - }, - new(values.Value), - ).Return([]types.Sequence{}, nil) - reader.EXPECT().QueryKey( - matches.AnyContext, - types.BoundContract{ - Name: WorkflowRegistryContractName, - Address: contractAddress, - }, - query.KeyFilter{ - Key: string(WorkflowActivatedEvent), - Expressions: []query.Expression{ - query.Confidence(primitives.Finalized), - query.Block("0", primitives.Gte), - }, - }, - query.LimitAndSort{ - SortBy: []query.SortBy{query.NewSortByTimestamp(query.Asc)}, - Limit: query.Limit{Count: giveCfg.QueryCount}, - }, - new(values.Value), - ).Return([]types.Sequence{}, nil) - reader.EXPECT().QueryKey( - matches.AnyContext, - types.BoundContract{ - Name: WorkflowRegistryContractName, - Address: contractAddress, - }, - query.KeyFilter{ - Key: string(WorkflowUpdatedEvent), - Expressions: []query.Expression{ - query.Confidence(primitives.Finalized), - query.Block("0", primitives.Gte), - }, - }, - query.LimitAndSort{ - SortBy: []query.SortBy{query.NewSortByTimestamp(query.Asc)}, - Limit: query.Limit{Count: giveCfg.QueryCount}, - }, - new(values.Value), - ).Return([]types.Sequence{}, nil) - reader.EXPECT().QueryKey( - matches.AnyContext, - types.BoundContract{ - Name: WorkflowRegistryContractName, - Address: contractAddress, - }, - query.KeyFilter{ - Key: string(WorkflowRegisteredEvent), - Expressions: []query.Expression{ - query.Confidence(primitives.Finalized), - query.Block("0", primitives.Gte), - }, - }, - query.LimitAndSort{ - SortBy: []query.SortBy{query.NewSortByTimestamp(query.Asc)}, - Limit: query.Limit{Count: giveCfg.QueryCount}, - }, - new(values.Value), - ).Return([]types.Sequence{}, nil) - reader.EXPECT().GetLatestValueWithHeadData(mock.Anything, mock.Anything, mock.Anything, mock.Anything, mock.Anything).Return(&types.Head{ - Height: "0", - }, nil) - reader.EXPECT().Start(mock.Anything).Return(nil) - reader.EXPECT().Bind(mock.Anything, mock.Anything).Return(nil) - - // Go run the worker - servicetest.Run(t, worker) - - // Send a tick to start a query - ticker <- time.Now() - - // Require the secrets contents to eventually be updated - require.Eventually(t, func() bool { - secrets, err := orm.GetContents(ctx, giveURL) - require.NoError(t, err) - return secrets == wantContents - }, 5*time.Second, time.Second) -}