From 0038065eb3ef6dbe013470d01aa16968104b85fc Mon Sep 17 00:00:00 2001 From: UlyanaAndrukhiv Date: Thu, 4 Dec 2025 17:50:05 +0200 Subject: [PATCH 01/15] Added validation checks for required executors IDs for optimistic sync --- engine/access/rpc/backend/common/errors.go | 47 +++++++++++++++++++ .../execution_result/info_provider.go | 38 ++++++++++++++- 2 files changed, 84 insertions(+), 1 deletion(-) diff --git a/engine/access/rpc/backend/common/errors.go b/engine/access/rpc/backend/common/errors.go index 51de9fbfcba..6b5504f416b 100644 --- a/engine/access/rpc/backend/common/errors.go +++ b/engine/access/rpc/backend/common/errors.go @@ -27,3 +27,50 @@ func IsInsufficientExecutionReceipts(err error) bool { var errInsufficientExecutionReceipts InsufficientExecutionReceipts return errors.As(err, &errInsufficientExecutionReceipts) } + +// RequiredExecutorsCountExceeded indicates that the requested number of required executors +// exceeds the total available execution nodes. +type RequiredExecutorsCountExceeded struct { + requiredExecutorsCount int + availableExecutorsCount int +} + +func NewRequiredExecutorsCountExceeded(requiredExecutorsCount int, availableExecutorsCount int) *RequiredExecutorsCountExceeded { + return &RequiredExecutorsCountExceeded{ + requiredExecutorsCount: requiredExecutorsCount, + availableExecutorsCount: availableExecutorsCount, + } +} + +func (e *RequiredExecutorsCountExceeded) Error() string { + return fmt.Sprintf( + "required executors count exceeded: required %d, available %d", + e.requiredExecutorsCount, e.availableExecutorsCount, + ) +} + +func IsRequiredExecutorsCountExceeded(err error) bool { + var target *RequiredExecutorsCountExceeded + return errors.As(err, &target) +} + +// UnknownRequiredExecutor indicates that a required executor ID is not present +// in the list of active execution nodes. +type UnknownRequiredExecutor struct { + executorID flow.Identifier +} + +func NewUnknownRequiredExecutor(executorID flow.Identifier) *UnknownRequiredExecutor { + return &UnknownRequiredExecutor{ + executorID: executorID, + } +} + +func (e *UnknownRequiredExecutor) Error() string { + return fmt.Sprintf("unknown required executor ID: %s", e.executorID.String()) +} + +func IsUnknownRequiredExecutor(err error) bool { + var target *UnknownRequiredExecutor + return errors.As(err, &target) +} diff --git a/module/executiondatasync/optimistic_sync/execution_result/info_provider.go b/module/executiondatasync/optimistic_sync/execution_result/info_provider.go index 9657d1cc565..b94c69207f5 100644 --- a/module/executiondatasync/optimistic_sync/execution_result/info_provider.go +++ b/module/executiondatasync/optimistic_sync/execution_result/info_provider.go @@ -53,6 +53,8 @@ func NewExecutionResultInfoProvider( // - [common.InsufficientExecutionReceipts]: Found insufficient receipts for given block ID. // - [storage.ErrNotFound]: If the request is for the spork root block and the node was bootstrapped // from a newer block. +// - [common.RequiredExecutorsCountExceeded]: Required executor IDs count exceeds available executors. +// - [common.UnknownRequiredExecutor]: A required executor ID is not in the available set. func (e *Provider) ExecutionResultInfo( blockID flow.Identifier, criteria optimistic_sync.Criteria, @@ -62,6 +64,11 @@ func (e *Provider) ExecutionResultInfo( return nil, fmt.Errorf("failed to retrieve execution IDs: %w", err) } + err = e.validateRequiredExecutors(criteria.RequiredExecutors, executorIdentities) + if err != nil { + return nil, fmt.Errorf("invalid required executors: %w", err) + } + // if the block ID is the root block, then use the root ExecutionResult and skip the receipt // check since there will not be any. if e.rootBlockID == blockID { @@ -112,12 +119,41 @@ func (e *Provider) ExecutionResultInfo( }, nil } +// validateRequiredExecutors verifies that the provided set of execution node +// identities contains all nodes required for processing and that the requested +// number of required executors does not exceed the available number. +// +// Expected errors during normal operations: +// - [common.RequiredExecutorsCountExceeded]: Required executor IDs count exceeds available executors. +// - [common.UnknownRequiredExecutor]: A required executor ID is not in the available set. +func (e *Provider) validateRequiredExecutors( + required flow.IdentifierList, + available flow.IdentityList, +) error { + if len(available) < len(required) { + return common.NewRequiredExecutorsCountExceeded( + len(required), + len(available), + ) + } + + lookup := available.Lookup() + for _, executorID := range required { + if _, ok := lookup[executorID]; !ok { + return common.NewUnknownRequiredExecutor(executorID) + } + } + + return nil +} + // findResultAndExecutors returns a query response for a given block ID. // The result must match the provided criteria and have at least one acceptable executor. If multiple // results are found, then the result with the most executors is returned. // // Expected errors during normal operations: -// - [common.InsufficientExecutionReceipts]: Found insufficient receipts for given block ID. +// - [common.MissingRequiredExecutor]: One or more required executors are not present. +// - [common.InsufficientExecutors]: The number of available executors is below the required minimum. func (e *Provider) findResultAndExecutors( blockID flow.Identifier, criteria optimistic_sync.Criteria, From dab885291f44f65ec69499a428b3933d61c5a7e1 Mon Sep 17 00:00:00 2001 From: UlyanaAndrukhiv Date: Fri, 5 Dec 2025 12:46:08 +0200 Subject: [PATCH 02/15] Added error handing. Added tests --- .../access/rpc/backend/accounts/accounts.go | 32 +++++++++ engine/access/rpc/backend/scripts/scripts.go | 12 ++++ engine/access/state_stream/backend/backend.go | 4 ++ .../backend/backend_executiondata.go | 4 ++ .../execution_result/info_provider_test.go | 72 +++++++++++++++---- 5 files changed, 110 insertions(+), 14 deletions(-) diff --git a/engine/access/rpc/backend/accounts/accounts.go b/engine/access/rpc/backend/accounts/accounts.go index f46a49a4aeb..c255552bd40 100644 --- a/engine/access/rpc/backend/accounts/accounts.go +++ b/engine/access/rpc/backend/accounts/accounts.go @@ -127,6 +127,10 @@ func (a *Accounts) GetAccountAtLatestBlock(ctx context.Context, address flow.Add return nil, nil, access.NewDataNotFoundError("execution data", err) case common.IsInsufficientExecutionReceipts(err): return nil, nil, access.NewDataNotFoundError("execution data", err) + case common.IsRequiredExecutorsCountExceeded(err): + return nil, nil, access.NewInvalidRequestError(err) + case common.IsUnknownRequiredExecutor(err): + return nil, nil, access.NewInvalidRequestError(err) default: return nil, nil, access.RequireNoError(ctx, err) } @@ -181,6 +185,10 @@ func (a *Accounts) GetAccountAtBlockHeight( return nil, nil, access.NewDataNotFoundError("execution data", err) case common.IsInsufficientExecutionReceipts(err): return nil, nil, access.NewDataNotFoundError("execution data", err) + case common.IsRequiredExecutorsCountExceeded(err): + return nil, nil, access.NewInvalidRequestError(err) + case common.IsUnknownRequiredExecutor(err): + return nil, nil, access.NewInvalidRequestError(err) default: return nil, nil, access.RequireNoError(ctx, err) } @@ -231,6 +239,10 @@ func (a *Accounts) GetAccountBalanceAtLatestBlock(ctx context.Context, address f return 0, nil, access.NewDataNotFoundError("execution data", err) case common.IsInsufficientExecutionReceipts(err): return 0, nil, access.NewDataNotFoundError("execution data", err) + case common.IsRequiredExecutorsCountExceeded(err): + return 0, nil, access.NewInvalidRequestError(err) + case common.IsUnknownRequiredExecutor(err): + return 0, nil, access.NewInvalidRequestError(err) default: return 0, nil, access.RequireNoError(ctx, err) } @@ -285,6 +297,10 @@ func (a *Accounts) GetAccountBalanceAtBlockHeight( return 0, nil, access.NewDataNotFoundError("execution data", err) case common.IsInsufficientExecutionReceipts(err): return 0, nil, access.NewDataNotFoundError("execution data", err) + case common.IsRequiredExecutorsCountExceeded(err): + return 0, nil, access.NewInvalidRequestError(err) + case common.IsUnknownRequiredExecutor(err): + return 0, nil, access.NewInvalidRequestError(err) default: return 0, nil, access.RequireNoError(ctx, err) } @@ -339,6 +355,10 @@ func (a *Accounts) GetAccountKeyAtLatestBlock( return nil, nil, access.NewDataNotFoundError("execution data", err) case common.IsInsufficientExecutionReceipts(err): return nil, nil, access.NewDataNotFoundError("execution data", err) + case common.IsRequiredExecutorsCountExceeded(err): + return nil, nil, access.NewInvalidRequestError(err) + case common.IsUnknownRequiredExecutor(err): + return nil, nil, access.NewInvalidRequestError(err) default: return nil, nil, access.RequireNoError(ctx, err) } @@ -394,6 +414,10 @@ func (a *Accounts) GetAccountKeyAtBlockHeight( return nil, nil, access.NewDataNotFoundError("execution data", err) case common.IsInsufficientExecutionReceipts(err): return nil, nil, access.NewDataNotFoundError("execution data", err) + case common.IsRequiredExecutorsCountExceeded(err): + return nil, nil, access.NewInvalidRequestError(err) + case common.IsUnknownRequiredExecutor(err): + return nil, nil, access.NewInvalidRequestError(err) default: return nil, nil, access.RequireNoError(ctx, err) } @@ -447,6 +471,10 @@ func (a *Accounts) GetAccountKeysAtLatestBlock( return nil, nil, access.NewDataNotFoundError("execution data", err) case common.IsInsufficientExecutionReceipts(err): return nil, nil, access.NewDataNotFoundError("execution data", err) + case common.IsRequiredExecutorsCountExceeded(err): + return nil, nil, access.NewInvalidRequestError(err) + case common.IsUnknownRequiredExecutor(err): + return nil, nil, access.NewInvalidRequestError(err) default: return nil, nil, access.RequireNoError(ctx, err) } @@ -501,6 +529,10 @@ func (a *Accounts) GetAccountKeysAtBlockHeight( return nil, nil, access.NewDataNotFoundError("execution data", err) case common.IsInsufficientExecutionReceipts(err): return nil, nil, access.NewDataNotFoundError("execution data", err) + case common.IsRequiredExecutorsCountExceeded(err): + return nil, nil, access.NewInvalidRequestError(err) + case common.IsUnknownRequiredExecutor(err): + return nil, nil, access.NewInvalidRequestError(err) default: return nil, nil, access.RequireNoError(ctx, err) } diff --git a/engine/access/rpc/backend/scripts/scripts.go b/engine/access/rpc/backend/scripts/scripts.go index f4b8f1ab927..11593840ee1 100644 --- a/engine/access/rpc/backend/scripts/scripts.go +++ b/engine/access/rpc/backend/scripts/scripts.go @@ -163,6 +163,10 @@ func (b *Scripts) ExecuteScriptAtLatestBlock( return nil, nil, access.NewDataNotFoundError("execution data", err) case common.IsInsufficientExecutionReceipts(err): return nil, nil, access.NewDataNotFoundError("execution data", err) + case common.IsRequiredExecutorsCountExceeded(err): + return nil, nil, access.NewInvalidRequestError(err) + case common.IsUnknownRequiredExecutor(err): + return nil, nil, access.NewInvalidRequestError(err) default: return nil, nil, access.RequireNoError(ctx, err) } @@ -221,6 +225,10 @@ func (b *Scripts) ExecuteScriptAtBlockID( return nil, nil, access.NewDataNotFoundError("execution data", err) case common.IsInsufficientExecutionReceipts(err): return nil, nil, access.NewDataNotFoundError("execution data", err) + case common.IsRequiredExecutorsCountExceeded(err): + return nil, nil, access.NewInvalidRequestError(err) + case common.IsUnknownRequiredExecutor(err): + return nil, nil, access.NewInvalidRequestError(err) default: return nil, nil, access.RequireNoError(ctx, err) } @@ -284,6 +292,10 @@ func (b *Scripts) ExecuteScriptAtBlockHeight( return nil, nil, access.NewDataNotFoundError("execution data", err) case common.IsInsufficientExecutionReceipts(err): return nil, nil, access.NewDataNotFoundError("execution data", err) + case common.IsRequiredExecutorsCountExceeded(err): + return nil, nil, access.NewInvalidRequestError(err) + case common.IsUnknownRequiredExecutor(err): + return nil, nil, access.NewInvalidRequestError(err) default: return nil, nil, access.RequireNoError(ctx, err) } diff --git a/engine/access/state_stream/backend/backend.go b/engine/access/state_stream/backend/backend.go index a4d5e75e778..15b7ab6315f 100644 --- a/engine/access/state_stream/backend/backend.go +++ b/engine/access/state_stream/backend/backend.go @@ -223,6 +223,10 @@ func (b *StateStreamBackend) GetRegisterValues( return nil, nil, access.NewDataNotFoundError("execution data", err) case common.IsInsufficientExecutionReceipts(err): return nil, nil, access.NewDataNotFoundError("execution data", err) + case common.IsRequiredExecutorsCountExceeded(err): + return nil, nil, access.NewInvalidRequestError(err) + case common.IsUnknownRequiredExecutor(err): + return nil, nil, access.NewInvalidRequestError(err) default: return nil, nil, access.RequireNoError(ctx, err) } diff --git a/engine/access/state_stream/backend/backend_executiondata.go b/engine/access/state_stream/backend/backend_executiondata.go index 7c9d2fd49df..4751b8ab888 100644 --- a/engine/access/state_stream/backend/backend_executiondata.go +++ b/engine/access/state_stream/backend/backend_executiondata.go @@ -61,6 +61,10 @@ func (b *ExecutionDataBackend) GetExecutionDataByBlockID( return nil, nil, access.NewDataNotFoundError("execution data", err) case common.IsInsufficientExecutionReceipts(err): return nil, nil, access.NewDataNotFoundError("execution data", err) + case common.IsRequiredExecutorsCountExceeded(err): + return nil, nil, access.NewInvalidRequestError(err) + case common.IsUnknownRequiredExecutor(err): + return nil, nil, access.NewInvalidRequestError(err) default: return nil, nil, access.RequireNoError(ctx, err) } diff --git a/module/executiondatasync/optimistic_sync/execution_result/info_provider_test.go b/module/executiondatasync/optimistic_sync/execution_result/info_provider_test.go index 035fc22ff6d..0f10f08cc28 100644 --- a/module/executiondatasync/optimistic_sync/execution_result/info_provider_test.go +++ b/module/executiondatasync/optimistic_sync/execution_result/info_provider_test.go @@ -75,7 +75,7 @@ func (suite *ExecutionResultInfoProviderSuite) setupIdentitiesMock(allExecutionN return allExecutionNodes.Filter(filter) }, func(flow.IdentityFilter[flow.Identity]) error { return nil }, - ) + ).Once() } // TestExecutionResultQuery tests the main ExecutionResult function with various scenarios. @@ -104,7 +104,7 @@ func (suite *ExecutionResultInfoProviderSuite) TestExecutionResultQuery() { receipts[i] = r } - suite.receipts.On("ByBlockID", block.ID()).Return(receipts, nil) + suite.receipts.On("ByBlockID", block.ID()).Return(receipts, nil).Once() suite.setupIdentitiesMock(allExecutionNodes) // Require specific executors (first two nodes) @@ -148,7 +148,7 @@ func (suite *ExecutionResultInfoProviderSuite) TestExecutionResultQuery() { receipts[i] = r } - suite.receipts.On("ByBlockID", block.ID()).Return(receipts, nil) + suite.receipts.On("ByBlockID", block.ID()).Return(receipts, nil).Once() suite.setupIdentitiesMock(allExecutionNodes) query, err := provider.ExecutionResultInfo(block.ID(), optimistic_sync.Criteria{}) @@ -180,14 +180,14 @@ func (suite *ExecutionResultInfoProviderSuite) TestExecutionResultQuery() { suite.receipts.On("ByBlockID", insufficientBlock.ID()).Return(receipts, nil).Once() suite.setupIdentitiesMock(allExecutionNodes) - _, err := provider.ExecutionResultInfo( + result, err := provider.ExecutionResultInfo( insufficientBlock.ID(), optimistic_sync.Criteria{ AgreeingExecutorsCount: 2, RequiredExecutors: allExecutionNodes[0:1].NodeIDs(), }, ) suite.Require().Error(err) - + suite.Require().Nil(result) suite.Assert().True(common.IsInsufficientExecutionReceipts(err)) }, ) @@ -198,7 +198,7 @@ func (suite *ExecutionResultInfoProviderSuite) TestExecutionResultQuery() { receipts := make(flow.ExecutionReceiptList, totalReceipts) for i := 0; i < totalReceipts; i++ { r := unittest.ReceiptForBlockFixture(block) - r.ExecutorID = allExecutionNodes[i].NodeID + r.ExecutorID = allExecutionNodes[0].NodeID r.ExecutionResult = *executionResult receipts[i] = r } @@ -207,28 +207,63 @@ func (suite *ExecutionResultInfoProviderSuite) TestExecutionResultQuery() { suite.setupIdentitiesMock(allExecutionNodes) // Require executors that didn't produce any receipts - _, err := provider.ExecutionResultInfo( + result, err := provider.ExecutionResultInfo( block.ID(), optimistic_sync.Criteria{ - RequiredExecutors: unittest.IdentityListFixture( - 2, - unittest.WithRole(flow.RoleExecution), - ).NodeIDs(), + RequiredExecutors: allExecutionNodes[1:2].NodeIDs(), }, ) suite.Require().Error(err) - + suite.Require().Nil(result) suite.Assert().True(common.IsInsufficientExecutionReceipts(err)) }, ) + + suite.Run("required executors count is greater than available executors count returns error", func() { + provider := suite.createProvider(flow.IdentifierList{}, optimistic_sync.Criteria{}) + + // setup specific executors (first two nodes) + suite.setupIdentitiesMock(allExecutionNodes[0:2]) + requiredExecutors := allExecutionNodes.NodeIDs() + + query, err := provider.ExecutionResultInfo( + block.ID(), optimistic_sync.Criteria{ + AgreeingExecutorsCount: 2, + RequiredExecutors: requiredExecutors, + }, + ) + suite.Require().Error(err) + suite.Require().Nil(query) + suite.Require().True(common.IsRequiredExecutorsCountExceeded(err)) + }) + + suite.Run("unknown required executor returns error", func() { + provider := suite.createProvider(flow.IdentifierList{}, optimistic_sync.Criteria{}) + + suite.setupIdentitiesMock(allExecutionNodes) + + unknownExecutorID := unittest.IdentifierFixture() + requiredExecutors := allExecutionNodes[0:1].NodeIDs() + requiredExecutors = append(requiredExecutors, unknownExecutorID) + + query, err := provider.ExecutionResultInfo( + block.ID(), optimistic_sync.Criteria{ + AgreeingExecutorsCount: 2, + RequiredExecutors: requiredExecutors, + }, + ) + suite.Require().Error(err) + suite.Require().Nil(query) + suite.Require().True(common.IsUnknownRequiredExecutor(err)) + }) } // TestRootBlockHandling tests the special case handling for root blocks. func (suite *ExecutionResultInfoProviderSuite) TestRootBlockHandling() { allExecutionNodes := unittest.IdentityListFixture(5, unittest.WithRole(flow.RoleExecution)) - suite.setupIdentitiesMock(allExecutionNodes) suite.Run( "root block returns execution nodes without execution result", func() { + suite.setupIdentitiesMock(allExecutionNodes) provider := suite.createProvider(flow.IdentifierList{}, optimistic_sync.Criteria{}) query, err := provider.ExecutionResultInfo( @@ -245,6 +280,7 @@ func (suite *ExecutionResultInfoProviderSuite) TestRootBlockHandling() { suite.Run( "root block with required executors", func() { + suite.setupIdentitiesMock(allExecutionNodes) provider := suite.createProvider(flow.IdentifierList{}, optimistic_sync.Criteria{}) requiredExecutors := allExecutionNodes[0:2].NodeIDs() @@ -279,10 +315,11 @@ func (suite *ExecutionResultInfoProviderSuite) TestPreferredAndRequiredExecution } suite.receipts.On("ByBlockID", block.ID()).Return(receipts, nil) - suite.setupIdentitiesMock(allExecutionNodes) suite.Run( "with default optimistic_sync.Criteria", func() { + suite.setupIdentitiesMock(allExecutionNodes) + provider := suite.createProvider(flow.IdentifierList{}, optimistic_sync.Criteria{}) // optimistic_sync.Criteria are empty to use operator defaults @@ -299,6 +336,8 @@ func (suite *ExecutionResultInfoProviderSuite) TestPreferredAndRequiredExecution suite.Run( "with operator preferred executors", func() { + suite.setupIdentitiesMock(allExecutionNodes) + provider := suite.createProvider( allExecutionNodes[1:5].NodeIDs(), optimistic_sync.Criteria{}, @@ -319,6 +358,8 @@ func (suite *ExecutionResultInfoProviderSuite) TestPreferredAndRequiredExecution suite.Run( "with operator required executors", func() { + suite.setupIdentitiesMock(allExecutionNodes) + provider := suite.createProvider( flow.IdentifierList{}, optimistic_sync.Criteria{ RequiredExecutors: allExecutionNodes[5:8].NodeIDs(), @@ -340,6 +381,7 @@ func (suite *ExecutionResultInfoProviderSuite) TestPreferredAndRequiredExecution suite.Run( "with both: operator preferred & required executors", func() { + suite.setupIdentitiesMock(allExecutionNodes) provider := suite.createProvider( allExecutionNodes[0:1].NodeIDs(), optimistic_sync.Criteria{ RequiredExecutors: allExecutionNodes[3:6].NodeIDs(), @@ -364,6 +406,8 @@ func (suite *ExecutionResultInfoProviderSuite) TestPreferredAndRequiredExecution suite.Run( "with client preferred executors", func() { + suite.setupIdentitiesMock(allExecutionNodes) + provider := suite.createProvider( allExecutionNodes[0:1].NodeIDs(), optimistic_sync.Criteria{ RequiredExecutors: allExecutionNodes[2:4].NodeIDs(), From aecdd153c33e163d4328ba8ef680be47a5b11489 Mon Sep 17 00:00:00 2001 From: UlyanaAndrukhiv Date: Fri, 5 Dec 2025 15:09:51 +0200 Subject: [PATCH 03/15] Added error handling when criteria for exec result for optimistic sync is mot met. Added tests. --- .../node_builder/access_node_builder.go | 1 + cmd/observer/node_builder/observer_builder.go | 1 + .../access/rpc/backend/accounts/accounts.go | 16 ++++++ engine/access/rpc/backend/common/errors.go | 25 ++++++++- engine/access/rpc/backend/scripts/scripts.go | 6 ++ engine/access/state_stream/backend/backend.go | 2 + .../backend/backend_executiondata.go | 2 + .../execution_result/info_provider.go | 19 ++++++- .../execution_result/info_provider_test.go | 56 ++++++++++++++++++- .../execution_result_info_provider.go | 5 +- 10 files changed, 125 insertions(+), 8 deletions(-) diff --git a/cmd/access/node_builder/access_node_builder.go b/cmd/access/node_builder/access_node_builder.go index ef41af946a9..70f084220b4 100644 --- a/cmd/access/node_builder/access_node_builder.go +++ b/cmd/access/node_builder/access_node_builder.go @@ -1777,6 +1777,7 @@ func (builder *FlowAccessNodeBuilder) buildExecutionResultInfoProvider() *FlowAc node.Logger, node.State, node.Storage.Receipts, + node.Storage.Headers, execNodeSelector, operatorCriteria, ) diff --git a/cmd/observer/node_builder/observer_builder.go b/cmd/observer/node_builder/observer_builder.go index a9d96fe9424..b14c389c3ce 100644 --- a/cmd/observer/node_builder/observer_builder.go +++ b/cmd/observer/node_builder/observer_builder.go @@ -1142,6 +1142,7 @@ func (builder *ObserverServiceBuilder) buildExecutionResultInfoProvider() *Obser node.Logger, node.State, node.Storage.Receipts, + node.Storage.Headers, execNodeSelector, operatorCriteria, ) diff --git a/engine/access/rpc/backend/accounts/accounts.go b/engine/access/rpc/backend/accounts/accounts.go index c255552bd40..d5500e76a8c 100644 --- a/engine/access/rpc/backend/accounts/accounts.go +++ b/engine/access/rpc/backend/accounts/accounts.go @@ -131,6 +131,8 @@ func (a *Accounts) GetAccountAtLatestBlock(ctx context.Context, address flow.Add return nil, nil, access.NewInvalidRequestError(err) case common.IsUnknownRequiredExecutor(err): return nil, nil, access.NewInvalidRequestError(err) + case common.IsCriteriaNotMetError(err): + return nil, nil, access.NewPreconditionFailedError(err) default: return nil, nil, access.RequireNoError(ctx, err) } @@ -189,6 +191,8 @@ func (a *Accounts) GetAccountAtBlockHeight( return nil, nil, access.NewInvalidRequestError(err) case common.IsUnknownRequiredExecutor(err): return nil, nil, access.NewInvalidRequestError(err) + case common.IsCriteriaNotMetError(err): + return nil, nil, access.NewPreconditionFailedError(err) default: return nil, nil, access.RequireNoError(ctx, err) } @@ -243,6 +247,8 @@ func (a *Accounts) GetAccountBalanceAtLatestBlock(ctx context.Context, address f return 0, nil, access.NewInvalidRequestError(err) case common.IsUnknownRequiredExecutor(err): return 0, nil, access.NewInvalidRequestError(err) + case common.IsCriteriaNotMetError(err): + return 0, nil, access.NewPreconditionFailedError(err) default: return 0, nil, access.RequireNoError(ctx, err) } @@ -301,6 +307,8 @@ func (a *Accounts) GetAccountBalanceAtBlockHeight( return 0, nil, access.NewInvalidRequestError(err) case common.IsUnknownRequiredExecutor(err): return 0, nil, access.NewInvalidRequestError(err) + case common.IsCriteriaNotMetError(err): + return 0, nil, access.NewPreconditionFailedError(err) default: return 0, nil, access.RequireNoError(ctx, err) } @@ -359,6 +367,8 @@ func (a *Accounts) GetAccountKeyAtLatestBlock( return nil, nil, access.NewInvalidRequestError(err) case common.IsUnknownRequiredExecutor(err): return nil, nil, access.NewInvalidRequestError(err) + case common.IsCriteriaNotMetError(err): + return nil, nil, access.NewPreconditionFailedError(err) default: return nil, nil, access.RequireNoError(ctx, err) } @@ -418,6 +428,8 @@ func (a *Accounts) GetAccountKeyAtBlockHeight( return nil, nil, access.NewInvalidRequestError(err) case common.IsUnknownRequiredExecutor(err): return nil, nil, access.NewInvalidRequestError(err) + case common.IsCriteriaNotMetError(err): + return nil, nil, access.NewPreconditionFailedError(err) default: return nil, nil, access.RequireNoError(ctx, err) } @@ -475,6 +487,8 @@ func (a *Accounts) GetAccountKeysAtLatestBlock( return nil, nil, access.NewInvalidRequestError(err) case common.IsUnknownRequiredExecutor(err): return nil, nil, access.NewInvalidRequestError(err) + case common.IsCriteriaNotMetError(err): + return nil, nil, access.NewPreconditionFailedError(err) default: return nil, nil, access.RequireNoError(ctx, err) } @@ -533,6 +547,8 @@ func (a *Accounts) GetAccountKeysAtBlockHeight( return nil, nil, access.NewInvalidRequestError(err) case common.IsUnknownRequiredExecutor(err): return nil, nil, access.NewInvalidRequestError(err) + case common.IsCriteriaNotMetError(err): + return nil, nil, access.NewPreconditionFailedError(err) default: return nil, nil, access.RequireNoError(ctx, err) } diff --git a/engine/access/rpc/backend/common/errors.go b/engine/access/rpc/backend/common/errors.go index 6b5504f416b..c75e47357c8 100644 --- a/engine/access/rpc/backend/common/errors.go +++ b/engine/access/rpc/backend/common/errors.go @@ -71,6 +71,27 @@ func (e *UnknownRequiredExecutor) Error() string { } func IsUnknownRequiredExecutor(err error) bool { - var target *UnknownRequiredExecutor - return errors.As(err, &target) + var unknownRequiredExecutor *UnknownRequiredExecutor + return errors.As(err, &unknownRequiredExecutor) +} + +// CriteriaNotMetError indicates that the execution result criteria could not be +// satisfied for a given block, when the block is already sealed. +type CriteriaNotMetError struct { + blockID flow.Identifier +} + +func NewCriteriaNotMetError(blockID flow.Identifier) *CriteriaNotMetError { + return &CriteriaNotMetError{ + blockID: blockID, + } +} + +func (e *CriteriaNotMetError) Error() string { + return fmt.Sprintf("block %s is already sealed but the criteria is still not met,", e.blockID) +} + +func IsCriteriaNotMetError(err error) bool { + var criteriaNotMetError *CriteriaNotMetError + return errors.As(err, &criteriaNotMetError) } diff --git a/engine/access/rpc/backend/scripts/scripts.go b/engine/access/rpc/backend/scripts/scripts.go index 11593840ee1..18d07f96a34 100644 --- a/engine/access/rpc/backend/scripts/scripts.go +++ b/engine/access/rpc/backend/scripts/scripts.go @@ -167,6 +167,8 @@ func (b *Scripts) ExecuteScriptAtLatestBlock( return nil, nil, access.NewInvalidRequestError(err) case common.IsUnknownRequiredExecutor(err): return nil, nil, access.NewInvalidRequestError(err) + case common.IsCriteriaNotMetError(err): + return nil, nil, access.NewPreconditionFailedError(err) default: return nil, nil, access.RequireNoError(ctx, err) } @@ -229,6 +231,8 @@ func (b *Scripts) ExecuteScriptAtBlockID( return nil, nil, access.NewInvalidRequestError(err) case common.IsUnknownRequiredExecutor(err): return nil, nil, access.NewInvalidRequestError(err) + case common.IsCriteriaNotMetError(err): + return nil, nil, access.NewPreconditionFailedError(err) default: return nil, nil, access.RequireNoError(ctx, err) } @@ -296,6 +300,8 @@ func (b *Scripts) ExecuteScriptAtBlockHeight( return nil, nil, access.NewInvalidRequestError(err) case common.IsUnknownRequiredExecutor(err): return nil, nil, access.NewInvalidRequestError(err) + case common.IsCriteriaNotMetError(err): + return nil, nil, access.NewPreconditionFailedError(err) default: return nil, nil, access.RequireNoError(ctx, err) } diff --git a/engine/access/state_stream/backend/backend.go b/engine/access/state_stream/backend/backend.go index 15b7ab6315f..6da8e3ba0f2 100644 --- a/engine/access/state_stream/backend/backend.go +++ b/engine/access/state_stream/backend/backend.go @@ -227,6 +227,8 @@ func (b *StateStreamBackend) GetRegisterValues( return nil, nil, access.NewInvalidRequestError(err) case common.IsUnknownRequiredExecutor(err): return nil, nil, access.NewInvalidRequestError(err) + case common.IsCriteriaNotMetError(err): + return nil, nil, access.NewPreconditionFailedError(err) default: return nil, nil, access.RequireNoError(ctx, err) } diff --git a/engine/access/state_stream/backend/backend_executiondata.go b/engine/access/state_stream/backend/backend_executiondata.go index 4751b8ab888..67b3506cb59 100644 --- a/engine/access/state_stream/backend/backend_executiondata.go +++ b/engine/access/state_stream/backend/backend_executiondata.go @@ -65,6 +65,8 @@ func (b *ExecutionDataBackend) GetExecutionDataByBlockID( return nil, nil, access.NewInvalidRequestError(err) case common.IsUnknownRequiredExecutor(err): return nil, nil, access.NewInvalidRequestError(err) + case common.IsCriteriaNotMetError(err): + return nil, nil, access.NewPreconditionFailedError(err) default: return nil, nil, access.RequireNoError(ctx, err) } diff --git a/module/executiondatasync/optimistic_sync/execution_result/info_provider.go b/module/executiondatasync/optimistic_sync/execution_result/info_provider.go index b94c69207f5..0d33e96db69 100644 --- a/module/executiondatasync/optimistic_sync/execution_result/info_provider.go +++ b/module/executiondatasync/optimistic_sync/execution_result/info_provider.go @@ -20,6 +20,7 @@ type Provider struct { log zerolog.Logger executionReceipts storage.ExecutionReceipts + headers storage.Headers state protocol.State rootBlockID flow.Identifier executionNodes *ExecutionNodeSelector @@ -33,12 +34,14 @@ func NewExecutionResultInfoProvider( log zerolog.Logger, state protocol.State, executionReceipts storage.ExecutionReceipts, + headers storage.Headers, executionNodes *ExecutionNodeSelector, operatorCriteria optimistic_sync.Criteria, ) *Provider { return &Provider{ log: log.With().Str("module", "execution_result_info").Logger(), executionReceipts: executionReceipts, + headers: headers, state: state, executionNodes: executionNodes, rootBlockID: state.Params().SporkRootBlock().ID(), @@ -51,8 +54,7 @@ func NewExecutionResultInfoProvider( // // Expected errors during normal operations: // - [common.InsufficientExecutionReceipts]: Found insufficient receipts for given block ID. -// - [storage.ErrNotFound]: If the request is for the spork root block and the node was bootstrapped -// from a newer block. +// - [storage.ErrNotFound]: If the data was not found. // - [common.RequiredExecutorsCountExceeded]: Required executor IDs count exceeds available executors. // - [common.UnknownRequiredExecutor]: A required executor ID is not in the available set. func (e *Provider) ExecutionResultInfo( @@ -101,6 +103,19 @@ func (e *Provider) ExecutionResultInfo( return nil, fmt.Errorf("failed to choose execution nodes for block ID %v: %w", blockID, err) } + sealedHeader, err := e.state.Sealed().Head() + if err != nil { + return nil, fmt.Errorf("failed to lookup sealed header: %w", err) + } + header, err := e.headers.ByBlockID(blockID) + if err != nil { + return nil, fmt.Errorf("failed to get header by block ID %v: %w", blockID, err) + } + // If block is sealed and criteria cannot be met return an error + if header.Height <= sealedHeader.Height && len(subsetENs) < len(criteria.RequiredExecutors) { + return nil, common.NewCriteriaNotMetError(blockID) + } + if len(subsetENs) == 0 { // this is unexpected, and probably indicates there is a bug. // There are only three ways that SelectExecutionNodes can return an empty list: diff --git a/module/executiondatasync/optimistic_sync/execution_result/info_provider_test.go b/module/executiondatasync/optimistic_sync/execution_result/info_provider_test.go index 0f10f08cc28..4c8b569da68 100644 --- a/module/executiondatasync/optimistic_sync/execution_result/info_provider_test.go +++ b/module/executiondatasync/optimistic_sync/execution_result/info_provider_test.go @@ -25,6 +25,7 @@ type ExecutionResultInfoProviderSuite struct { log zerolog.Logger receipts *storagemock.ExecutionReceipts + headers *storagemock.Headers rootBlock *flow.Block rootBlockResult *flow.ExecutionResult @@ -42,7 +43,7 @@ func (suite *ExecutionResultInfoProviderSuite) SetupTest() { suite.snapshot = protocol.NewSnapshot(t) suite.params = protocol.NewParams(t) suite.receipts = storagemock.NewExecutionReceipts(t) - + suite.headers = storagemock.NewHeaders(t) suite.rootBlock = unittest.BlockFixture() rootBlockID := suite.rootBlock.ID() suite.rootBlockResult = unittest.ExecutionResultFixture(unittest.WithExecutionResultBlockID(rootBlockID)) @@ -63,6 +64,7 @@ func (suite *ExecutionResultInfoProviderSuite) createProvider( suite.log, suite.state, suite.receipts, + suite.headers, NewExecutionNodeSelector(preferredExecutors, operatorCriteria.RequiredExecutors), operatorCriteria, ) @@ -105,6 +107,9 @@ func (suite *ExecutionResultInfoProviderSuite) TestExecutionResultQuery() { } suite.receipts.On("ByBlockID", block.ID()).Return(receipts, nil).Once() + suite.headers.On("ByBlockID", block.ID()).Return(block.ToHeader(), nil).Once() + suite.state.On("Sealed").Return(suite.snapshot, nil).Once() + suite.snapshot.On("Head").Return(func() *flow.Header { return block.ToHeader() }, nil).Once() suite.setupIdentitiesMock(allExecutionNodes) // Require specific executors (first two nodes) @@ -148,8 +153,11 @@ func (suite *ExecutionResultInfoProviderSuite) TestExecutionResultQuery() { receipts[i] = r } - suite.receipts.On("ByBlockID", block.ID()).Return(receipts, nil).Once() suite.setupIdentitiesMock(allExecutionNodes) + suite.receipts.On("ByBlockID", block.ID()).Return(receipts, nil).Once() + suite.headers.On("ByBlockID", block.ID()).Return(block.ToHeader(), nil).Once() + suite.state.On("Sealed").Return(suite.snapshot, nil).Once() + suite.snapshot.On("Head").Return(func() *flow.Header { return block.ToHeader() }, nil).Once() query, err := provider.ExecutionResultInfo(block.ID(), optimistic_sync.Criteria{}) suite.Require().NoError(err) @@ -255,6 +263,34 @@ func (suite *ExecutionResultInfoProviderSuite) TestExecutionResultQuery() { suite.Require().Nil(query) suite.Require().True(common.IsUnknownRequiredExecutor(err)) }) + + suite.Run("criteria not met on sealed block returns error", func() { + provider := suite.createProvider(flow.IdentifierList{}, optimistic_sync.Criteria{}) + + receipts := make(flow.ExecutionReceiptList, totalReceipts) + for i := 0; i < totalReceipts; i++ { + r := unittest.ReceiptForBlockFixture(block) + r.ExecutorID = allExecutionNodes[0].NodeID + r.ExecutionResult = *executionResult + receipts[i] = r + } + suite.receipts.On("ByBlockID", block.ID()).Return(receipts, nil).Once() + suite.headers.On("ByBlockID", block.ID()).Return(block.ToHeader(), nil).Once() + suite.state.On("Sealed").Return(suite.snapshot, nil).Once() + suite.snapshot.On("Head").Return(func() *flow.Header { return block.ToHeader() }, nil).Once() + suite.setupIdentitiesMock(allExecutionNodes) + + // Require all executors, but only one produces receipts + query, err := provider.ExecutionResultInfo( + block.ID(), optimistic_sync.Criteria{ + AgreeingExecutorsCount: 1, + RequiredExecutors: allExecutionNodes.NodeIDs(), + }, + ) + suite.Require().Error(err) + suite.Require().Nil(query) + suite.Require().True(common.IsCriteriaNotMetError(err)) + }) } // TestRootBlockHandling tests the special case handling for root blocks. @@ -318,6 +354,9 @@ func (suite *ExecutionResultInfoProviderSuite) TestPreferredAndRequiredExecution suite.Run( "with default optimistic_sync.Criteria", func() { + suite.headers.On("ByBlockID", block.ID()).Return(block.ToHeader(), nil).Once() + suite.state.On("Sealed").Return(suite.snapshot, nil).Once() + suite.snapshot.On("Head").Return(func() *flow.Header { return block.ToHeader() }, nil).Once() suite.setupIdentitiesMock(allExecutionNodes) provider := suite.createProvider(flow.IdentifierList{}, optimistic_sync.Criteria{}) @@ -336,6 +375,9 @@ func (suite *ExecutionResultInfoProviderSuite) TestPreferredAndRequiredExecution suite.Run( "with operator preferred executors", func() { + suite.headers.On("ByBlockID", block.ID()).Return(block.ToHeader(), nil).Once() + suite.state.On("Sealed").Return(suite.snapshot, nil).Once() + suite.snapshot.On("Head").Return(func() *flow.Header { return block.ToHeader() }, nil).Once() suite.setupIdentitiesMock(allExecutionNodes) provider := suite.createProvider( @@ -358,6 +400,9 @@ func (suite *ExecutionResultInfoProviderSuite) TestPreferredAndRequiredExecution suite.Run( "with operator required executors", func() { + suite.headers.On("ByBlockID", block.ID()).Return(block.ToHeader(), nil).Once() + suite.state.On("Sealed").Return(suite.snapshot, nil).Once() + suite.snapshot.On("Head").Return(func() *flow.Header { return block.ToHeader() }, nil).Once() suite.setupIdentitiesMock(allExecutionNodes) provider := suite.createProvider( @@ -381,7 +426,11 @@ func (suite *ExecutionResultInfoProviderSuite) TestPreferredAndRequiredExecution suite.Run( "with both: operator preferred & required executors", func() { + suite.headers.On("ByBlockID", block.ID()).Return(block.ToHeader(), nil).Once() + suite.state.On("Sealed").Return(suite.snapshot, nil).Once() + suite.snapshot.On("Head").Return(func() *flow.Header { return block.ToHeader() }, nil).Once() suite.setupIdentitiesMock(allExecutionNodes) + provider := suite.createProvider( allExecutionNodes[0:1].NodeIDs(), optimistic_sync.Criteria{ RequiredExecutors: allExecutionNodes[3:6].NodeIDs(), @@ -406,6 +455,9 @@ func (suite *ExecutionResultInfoProviderSuite) TestPreferredAndRequiredExecution suite.Run( "with client preferred executors", func() { + suite.headers.On("ByBlockID", block.ID()).Return(block.ToHeader(), nil).Once() + suite.state.On("Sealed").Return(suite.snapshot, nil).Once() + suite.snapshot.On("Head").Return(func() *flow.Header { return block.ToHeader() }, nil).Once() suite.setupIdentitiesMock(allExecutionNodes) provider := suite.createProvider( diff --git a/module/executiondatasync/optimistic_sync/execution_result_info_provider.go b/module/executiondatasync/optimistic_sync/execution_result_info_provider.go index 4a47b31a0d6..3d959d9a0bd 100644 --- a/module/executiondatasync/optimistic_sync/execution_result_info_provider.go +++ b/module/executiondatasync/optimistic_sync/execution_result_info_provider.go @@ -54,7 +54,8 @@ type ExecutionResultInfoProvider interface { // // Expected error returns during normal operation: // - [common.InsufficientExecutionReceipts]: Found insufficient receipts for given block ID. - // - [storage.ErrNotFound]: If the request is for the spork root block and the node was bootstrapped - // from a newer block. + // - [storage.ErrNotFound]: If the data was not found. + // - [common.RequiredExecutorsCountExceeded]: Required executor IDs count exceeds available executors. + // - [common.UnknownRequiredExecutor]: A required executor ID is not in the available set. ExecutionResultInfo(blockID flow.Identifier, criteria Criteria) (*ExecutionResultInfo, error) } From c4f1593c2431149b32559c1b855609413ebff0e6 Mon Sep 17 00:00:00 2001 From: UlyanaAndrukhiv Date: Fri, 5 Dec 2025 15:20:36 +0200 Subject: [PATCH 04/15] Moved errors from common rpc to optimistic sync package --- .../access/rpc/backend/accounts/accounts.go | 48 ++++++------ engine/access/rpc/backend/common/errors.go | 68 ----------------- engine/access/rpc/backend/scripts/scripts.go | 18 ++--- engine/access/state_stream/backend/backend.go | 6 +- .../backend/backend_executiondata.go | 6 +- .../optimistic_sync/errors.go | 76 +++++++++++++++++++ .../execution_result/info_provider.go | 14 ++-- .../execution_result/info_provider_test.go | 6 +- .../execution_result_info_provider.go | 4 +- 9 files changed, 127 insertions(+), 119 deletions(-) create mode 100644 module/executiondatasync/optimistic_sync/errors.go diff --git a/engine/access/rpc/backend/accounts/accounts.go b/engine/access/rpc/backend/accounts/accounts.go index d5500e76a8c..cc090b3d0b1 100644 --- a/engine/access/rpc/backend/accounts/accounts.go +++ b/engine/access/rpc/backend/accounts/accounts.go @@ -127,11 +127,11 @@ func (a *Accounts) GetAccountAtLatestBlock(ctx context.Context, address flow.Add return nil, nil, access.NewDataNotFoundError("execution data", err) case common.IsInsufficientExecutionReceipts(err): return nil, nil, access.NewDataNotFoundError("execution data", err) - case common.IsRequiredExecutorsCountExceeded(err): + case optimistic_sync.IsRequiredExecutorsCountExceededError(err): return nil, nil, access.NewInvalidRequestError(err) - case common.IsUnknownRequiredExecutor(err): + case optimistic_sync.IsUnknownRequiredExecutorError(err): return nil, nil, access.NewInvalidRequestError(err) - case common.IsCriteriaNotMetError(err): + case optimistic_sync.IsCriteriaNotMetError(err): return nil, nil, access.NewPreconditionFailedError(err) default: return nil, nil, access.RequireNoError(ctx, err) @@ -187,11 +187,11 @@ func (a *Accounts) GetAccountAtBlockHeight( return nil, nil, access.NewDataNotFoundError("execution data", err) case common.IsInsufficientExecutionReceipts(err): return nil, nil, access.NewDataNotFoundError("execution data", err) - case common.IsRequiredExecutorsCountExceeded(err): + case optimistic_sync.IsRequiredExecutorsCountExceededError(err): return nil, nil, access.NewInvalidRequestError(err) - case common.IsUnknownRequiredExecutor(err): + case optimistic_sync.IsUnknownRequiredExecutorError(err): return nil, nil, access.NewInvalidRequestError(err) - case common.IsCriteriaNotMetError(err): + case optimistic_sync.IsCriteriaNotMetError(err): return nil, nil, access.NewPreconditionFailedError(err) default: return nil, nil, access.RequireNoError(ctx, err) @@ -243,11 +243,11 @@ func (a *Accounts) GetAccountBalanceAtLatestBlock(ctx context.Context, address f return 0, nil, access.NewDataNotFoundError("execution data", err) case common.IsInsufficientExecutionReceipts(err): return 0, nil, access.NewDataNotFoundError("execution data", err) - case common.IsRequiredExecutorsCountExceeded(err): + case optimistic_sync.IsRequiredExecutorsCountExceededError(err): return 0, nil, access.NewInvalidRequestError(err) - case common.IsUnknownRequiredExecutor(err): + case optimistic_sync.IsUnknownRequiredExecutorError(err): return 0, nil, access.NewInvalidRequestError(err) - case common.IsCriteriaNotMetError(err): + case optimistic_sync.IsCriteriaNotMetError(err): return 0, nil, access.NewPreconditionFailedError(err) default: return 0, nil, access.RequireNoError(ctx, err) @@ -303,11 +303,11 @@ func (a *Accounts) GetAccountBalanceAtBlockHeight( return 0, nil, access.NewDataNotFoundError("execution data", err) case common.IsInsufficientExecutionReceipts(err): return 0, nil, access.NewDataNotFoundError("execution data", err) - case common.IsRequiredExecutorsCountExceeded(err): + case optimistic_sync.IsRequiredExecutorsCountExceededError(err): return 0, nil, access.NewInvalidRequestError(err) - case common.IsUnknownRequiredExecutor(err): + case optimistic_sync.IsUnknownRequiredExecutorError(err): return 0, nil, access.NewInvalidRequestError(err) - case common.IsCriteriaNotMetError(err): + case optimistic_sync.IsCriteriaNotMetError(err): return 0, nil, access.NewPreconditionFailedError(err) default: return 0, nil, access.RequireNoError(ctx, err) @@ -363,11 +363,11 @@ func (a *Accounts) GetAccountKeyAtLatestBlock( return nil, nil, access.NewDataNotFoundError("execution data", err) case common.IsInsufficientExecutionReceipts(err): return nil, nil, access.NewDataNotFoundError("execution data", err) - case common.IsRequiredExecutorsCountExceeded(err): + case optimistic_sync.IsRequiredExecutorsCountExceededError(err): return nil, nil, access.NewInvalidRequestError(err) - case common.IsUnknownRequiredExecutor(err): + case optimistic_sync.IsUnknownRequiredExecutorError(err): return nil, nil, access.NewInvalidRequestError(err) - case common.IsCriteriaNotMetError(err): + case optimistic_sync.IsCriteriaNotMetError(err): return nil, nil, access.NewPreconditionFailedError(err) default: return nil, nil, access.RequireNoError(ctx, err) @@ -424,11 +424,11 @@ func (a *Accounts) GetAccountKeyAtBlockHeight( return nil, nil, access.NewDataNotFoundError("execution data", err) case common.IsInsufficientExecutionReceipts(err): return nil, nil, access.NewDataNotFoundError("execution data", err) - case common.IsRequiredExecutorsCountExceeded(err): + case optimistic_sync.IsRequiredExecutorsCountExceededError(err): return nil, nil, access.NewInvalidRequestError(err) - case common.IsUnknownRequiredExecutor(err): + case optimistic_sync.IsUnknownRequiredExecutorError(err): return nil, nil, access.NewInvalidRequestError(err) - case common.IsCriteriaNotMetError(err): + case optimistic_sync.IsCriteriaNotMetError(err): return nil, nil, access.NewPreconditionFailedError(err) default: return nil, nil, access.RequireNoError(ctx, err) @@ -483,11 +483,11 @@ func (a *Accounts) GetAccountKeysAtLatestBlock( return nil, nil, access.NewDataNotFoundError("execution data", err) case common.IsInsufficientExecutionReceipts(err): return nil, nil, access.NewDataNotFoundError("execution data", err) - case common.IsRequiredExecutorsCountExceeded(err): + case optimistic_sync.IsRequiredExecutorsCountExceededError(err): return nil, nil, access.NewInvalidRequestError(err) - case common.IsUnknownRequiredExecutor(err): + case optimistic_sync.IsUnknownRequiredExecutorError(err): return nil, nil, access.NewInvalidRequestError(err) - case common.IsCriteriaNotMetError(err): + case optimistic_sync.IsCriteriaNotMetError(err): return nil, nil, access.NewPreconditionFailedError(err) default: return nil, nil, access.RequireNoError(ctx, err) @@ -543,11 +543,11 @@ func (a *Accounts) GetAccountKeysAtBlockHeight( return nil, nil, access.NewDataNotFoundError("execution data", err) case common.IsInsufficientExecutionReceipts(err): return nil, nil, access.NewDataNotFoundError("execution data", err) - case common.IsRequiredExecutorsCountExceeded(err): + case optimistic_sync.IsRequiredExecutorsCountExceededError(err): return nil, nil, access.NewInvalidRequestError(err) - case common.IsUnknownRequiredExecutor(err): + case optimistic_sync.IsUnknownRequiredExecutorError(err): return nil, nil, access.NewInvalidRequestError(err) - case common.IsCriteriaNotMetError(err): + case optimistic_sync.IsCriteriaNotMetError(err): return nil, nil, access.NewPreconditionFailedError(err) default: return nil, nil, access.RequireNoError(ctx, err) diff --git a/engine/access/rpc/backend/common/errors.go b/engine/access/rpc/backend/common/errors.go index c75e47357c8..51de9fbfcba 100644 --- a/engine/access/rpc/backend/common/errors.go +++ b/engine/access/rpc/backend/common/errors.go @@ -27,71 +27,3 @@ func IsInsufficientExecutionReceipts(err error) bool { var errInsufficientExecutionReceipts InsufficientExecutionReceipts return errors.As(err, &errInsufficientExecutionReceipts) } - -// RequiredExecutorsCountExceeded indicates that the requested number of required executors -// exceeds the total available execution nodes. -type RequiredExecutorsCountExceeded struct { - requiredExecutorsCount int - availableExecutorsCount int -} - -func NewRequiredExecutorsCountExceeded(requiredExecutorsCount int, availableExecutorsCount int) *RequiredExecutorsCountExceeded { - return &RequiredExecutorsCountExceeded{ - requiredExecutorsCount: requiredExecutorsCount, - availableExecutorsCount: availableExecutorsCount, - } -} - -func (e *RequiredExecutorsCountExceeded) Error() string { - return fmt.Sprintf( - "required executors count exceeded: required %d, available %d", - e.requiredExecutorsCount, e.availableExecutorsCount, - ) -} - -func IsRequiredExecutorsCountExceeded(err error) bool { - var target *RequiredExecutorsCountExceeded - return errors.As(err, &target) -} - -// UnknownRequiredExecutor indicates that a required executor ID is not present -// in the list of active execution nodes. -type UnknownRequiredExecutor struct { - executorID flow.Identifier -} - -func NewUnknownRequiredExecutor(executorID flow.Identifier) *UnknownRequiredExecutor { - return &UnknownRequiredExecutor{ - executorID: executorID, - } -} - -func (e *UnknownRequiredExecutor) Error() string { - return fmt.Sprintf("unknown required executor ID: %s", e.executorID.String()) -} - -func IsUnknownRequiredExecutor(err error) bool { - var unknownRequiredExecutor *UnknownRequiredExecutor - return errors.As(err, &unknownRequiredExecutor) -} - -// CriteriaNotMetError indicates that the execution result criteria could not be -// satisfied for a given block, when the block is already sealed. -type CriteriaNotMetError struct { - blockID flow.Identifier -} - -func NewCriteriaNotMetError(blockID flow.Identifier) *CriteriaNotMetError { - return &CriteriaNotMetError{ - blockID: blockID, - } -} - -func (e *CriteriaNotMetError) Error() string { - return fmt.Sprintf("block %s is already sealed but the criteria is still not met,", e.blockID) -} - -func IsCriteriaNotMetError(err error) bool { - var criteriaNotMetError *CriteriaNotMetError - return errors.As(err, &criteriaNotMetError) -} diff --git a/engine/access/rpc/backend/scripts/scripts.go b/engine/access/rpc/backend/scripts/scripts.go index 18d07f96a34..e751ce19f73 100644 --- a/engine/access/rpc/backend/scripts/scripts.go +++ b/engine/access/rpc/backend/scripts/scripts.go @@ -163,11 +163,11 @@ func (b *Scripts) ExecuteScriptAtLatestBlock( return nil, nil, access.NewDataNotFoundError("execution data", err) case common.IsInsufficientExecutionReceipts(err): return nil, nil, access.NewDataNotFoundError("execution data", err) - case common.IsRequiredExecutorsCountExceeded(err): + case optimistic_sync.IsRequiredExecutorsCountExceededError(err): return nil, nil, access.NewInvalidRequestError(err) - case common.IsUnknownRequiredExecutor(err): + case optimistic_sync.IsUnknownRequiredExecutorError(err): return nil, nil, access.NewInvalidRequestError(err) - case common.IsCriteriaNotMetError(err): + case optimistic_sync.IsCriteriaNotMetError(err): return nil, nil, access.NewPreconditionFailedError(err) default: return nil, nil, access.RequireNoError(ctx, err) @@ -227,11 +227,11 @@ func (b *Scripts) ExecuteScriptAtBlockID( return nil, nil, access.NewDataNotFoundError("execution data", err) case common.IsInsufficientExecutionReceipts(err): return nil, nil, access.NewDataNotFoundError("execution data", err) - case common.IsRequiredExecutorsCountExceeded(err): + case optimistic_sync.IsRequiredExecutorsCountExceededError(err): return nil, nil, access.NewInvalidRequestError(err) - case common.IsUnknownRequiredExecutor(err): + case optimistic_sync.IsUnknownRequiredExecutorError(err): return nil, nil, access.NewInvalidRequestError(err) - case common.IsCriteriaNotMetError(err): + case optimistic_sync.IsCriteriaNotMetError(err): return nil, nil, access.NewPreconditionFailedError(err) default: return nil, nil, access.RequireNoError(ctx, err) @@ -296,11 +296,11 @@ func (b *Scripts) ExecuteScriptAtBlockHeight( return nil, nil, access.NewDataNotFoundError("execution data", err) case common.IsInsufficientExecutionReceipts(err): return nil, nil, access.NewDataNotFoundError("execution data", err) - case common.IsRequiredExecutorsCountExceeded(err): + case optimistic_sync.IsRequiredExecutorsCountExceededError(err): return nil, nil, access.NewInvalidRequestError(err) - case common.IsUnknownRequiredExecutor(err): + case optimistic_sync.IsUnknownRequiredExecutorError(err): return nil, nil, access.NewInvalidRequestError(err) - case common.IsCriteriaNotMetError(err): + case optimistic_sync.IsCriteriaNotMetError(err): return nil, nil, access.NewPreconditionFailedError(err) default: return nil, nil, access.RequireNoError(ctx, err) diff --git a/engine/access/state_stream/backend/backend.go b/engine/access/state_stream/backend/backend.go index 6da8e3ba0f2..c2ba6b9be8f 100644 --- a/engine/access/state_stream/backend/backend.go +++ b/engine/access/state_stream/backend/backend.go @@ -223,11 +223,11 @@ func (b *StateStreamBackend) GetRegisterValues( return nil, nil, access.NewDataNotFoundError("execution data", err) case common.IsInsufficientExecutionReceipts(err): return nil, nil, access.NewDataNotFoundError("execution data", err) - case common.IsRequiredExecutorsCountExceeded(err): + case optimistic_sync.IsRequiredExecutorsCountExceededError(err): return nil, nil, access.NewInvalidRequestError(err) - case common.IsUnknownRequiredExecutor(err): + case optimistic_sync.IsUnknownRequiredExecutorError(err): return nil, nil, access.NewInvalidRequestError(err) - case common.IsCriteriaNotMetError(err): + case optimistic_sync.IsCriteriaNotMetError(err): return nil, nil, access.NewPreconditionFailedError(err) default: return nil, nil, access.RequireNoError(ctx, err) diff --git a/engine/access/state_stream/backend/backend_executiondata.go b/engine/access/state_stream/backend/backend_executiondata.go index 67b3506cb59..2ef38f48ebd 100644 --- a/engine/access/state_stream/backend/backend_executiondata.go +++ b/engine/access/state_stream/backend/backend_executiondata.go @@ -61,11 +61,11 @@ func (b *ExecutionDataBackend) GetExecutionDataByBlockID( return nil, nil, access.NewDataNotFoundError("execution data", err) case common.IsInsufficientExecutionReceipts(err): return nil, nil, access.NewDataNotFoundError("execution data", err) - case common.IsRequiredExecutorsCountExceeded(err): + case optimistic_sync.IsRequiredExecutorsCountExceededError(err): return nil, nil, access.NewInvalidRequestError(err) - case common.IsUnknownRequiredExecutor(err): + case optimistic_sync.IsUnknownRequiredExecutorError(err): return nil, nil, access.NewInvalidRequestError(err) - case common.IsCriteriaNotMetError(err): + case optimistic_sync.IsCriteriaNotMetError(err): return nil, nil, access.NewPreconditionFailedError(err) default: return nil, nil, access.RequireNoError(ctx, err) diff --git a/module/executiondatasync/optimistic_sync/errors.go b/module/executiondatasync/optimistic_sync/errors.go new file mode 100644 index 00000000000..1181b3ae202 --- /dev/null +++ b/module/executiondatasync/optimistic_sync/errors.go @@ -0,0 +1,76 @@ +package optimistic_sync + +import ( + "errors" + "fmt" + + "github.com/onflow/flow-go/model/flow" +) + +// RequiredExecutorsCountExceededError indicates that the requested number of required executors +// exceeds the total available execution nodes. +type RequiredExecutorsCountExceededError struct { + requiredExecutorsCount int + availableExecutorsCount int +} + +func NewRequiredExecutorsCountExceededError(requiredExecutorsCount int, availableExecutorsCount int) *RequiredExecutorsCountExceededError { + return &RequiredExecutorsCountExceededError{ + requiredExecutorsCount: requiredExecutorsCount, + availableExecutorsCount: availableExecutorsCount, + } +} + +func (e *RequiredExecutorsCountExceededError) Error() string { + return fmt.Sprintf( + "required executors count exceeded: required %d, available %d", + e.requiredExecutorsCount, e.availableExecutorsCount, + ) +} + +func IsRequiredExecutorsCountExceededError(err error) bool { + var target *RequiredExecutorsCountExceededError + return errors.As(err, &target) +} + +// UnknownRequiredExecutorError indicates that a required executor ID is not present +// in the list of active execution nodes. +type UnknownRequiredExecutorError struct { + executorID flow.Identifier +} + +func NewUnknownRequiredExecutorError(executorID flow.Identifier) *UnknownRequiredExecutorError { + return &UnknownRequiredExecutorError{ + executorID: executorID, + } +} + +func (e *UnknownRequiredExecutorError) Error() string { + return fmt.Sprintf("unknown required executor ID: %s", e.executorID.String()) +} + +func IsUnknownRequiredExecutorError(err error) bool { + var unknownRequiredExecutor *UnknownRequiredExecutorError + return errors.As(err, &unknownRequiredExecutor) +} + +// CriteriaNotMetError indicates that the execution result criteria could not be +// satisfied for a given block, when the block is already sealed. +type CriteriaNotMetError struct { + blockID flow.Identifier +} + +func NewCriteriaNotMetError(blockID flow.Identifier) *CriteriaNotMetError { + return &CriteriaNotMetError{ + blockID: blockID, + } +} + +func (e *CriteriaNotMetError) Error() string { + return fmt.Sprintf("block %s is already sealed but the criteria is still not met,", e.blockID) +} + +func IsCriteriaNotMetError(err error) bool { + var criteriaNotMetError *CriteriaNotMetError + return errors.As(err, &criteriaNotMetError) +} diff --git a/module/executiondatasync/optimistic_sync/execution_result/info_provider.go b/module/executiondatasync/optimistic_sync/execution_result/info_provider.go index 0d33e96db69..7ee0f2a377b 100644 --- a/module/executiondatasync/optimistic_sync/execution_result/info_provider.go +++ b/module/executiondatasync/optimistic_sync/execution_result/info_provider.go @@ -55,8 +55,8 @@ func NewExecutionResultInfoProvider( // Expected errors during normal operations: // - [common.InsufficientExecutionReceipts]: Found insufficient receipts for given block ID. // - [storage.ErrNotFound]: If the data was not found. -// - [common.RequiredExecutorsCountExceeded]: Required executor IDs count exceeds available executors. -// - [common.UnknownRequiredExecutor]: A required executor ID is not in the available set. +// - [optimistic_sync.RequiredExecutorsCountExceededError]: Required executor IDs count exceeds available executors. +// - [optimistic_sync.UnknownRequiredExecutorError]: A required executor ID is not in the available set. func (e *Provider) ExecutionResultInfo( blockID flow.Identifier, criteria optimistic_sync.Criteria, @@ -113,7 +113,7 @@ func (e *Provider) ExecutionResultInfo( } // If block is sealed and criteria cannot be met return an error if header.Height <= sealedHeader.Height && len(subsetENs) < len(criteria.RequiredExecutors) { - return nil, common.NewCriteriaNotMetError(blockID) + return nil, optimistic_sync.NewCriteriaNotMetError(blockID) } if len(subsetENs) == 0 { @@ -139,14 +139,14 @@ func (e *Provider) ExecutionResultInfo( // number of required executors does not exceed the available number. // // Expected errors during normal operations: -// - [common.RequiredExecutorsCountExceeded]: Required executor IDs count exceeds available executors. -// - [common.UnknownRequiredExecutor]: A required executor ID is not in the available set. +// - [common.RequiredExecutorsCountExceededError]: Required executor IDs count exceeds available executors. +// - [common.UnknownRequiredExecutorError]: A required executor ID is not in the available set. func (e *Provider) validateRequiredExecutors( required flow.IdentifierList, available flow.IdentityList, ) error { if len(available) < len(required) { - return common.NewRequiredExecutorsCountExceeded( + return optimistic_sync.NewRequiredExecutorsCountExceededError( len(required), len(available), ) @@ -155,7 +155,7 @@ func (e *Provider) validateRequiredExecutors( lookup := available.Lookup() for _, executorID := range required { if _, ok := lookup[executorID]; !ok { - return common.NewUnknownRequiredExecutor(executorID) + return optimistic_sync.NewUnknownRequiredExecutorError(executorID) } } diff --git a/module/executiondatasync/optimistic_sync/execution_result/info_provider_test.go b/module/executiondatasync/optimistic_sync/execution_result/info_provider_test.go index 4c8b569da68..a513a66dfb9 100644 --- a/module/executiondatasync/optimistic_sync/execution_result/info_provider_test.go +++ b/module/executiondatasync/optimistic_sync/execution_result/info_provider_test.go @@ -241,7 +241,7 @@ func (suite *ExecutionResultInfoProviderSuite) TestExecutionResultQuery() { ) suite.Require().Error(err) suite.Require().Nil(query) - suite.Require().True(common.IsRequiredExecutorsCountExceeded(err)) + suite.Require().True(optimistic_sync.IsRequiredExecutorsCountExceededError(err)) }) suite.Run("unknown required executor returns error", func() { @@ -261,7 +261,7 @@ func (suite *ExecutionResultInfoProviderSuite) TestExecutionResultQuery() { ) suite.Require().Error(err) suite.Require().Nil(query) - suite.Require().True(common.IsUnknownRequiredExecutor(err)) + suite.Require().True(optimistic_sync.IsUnknownRequiredExecutorError(err)) }) suite.Run("criteria not met on sealed block returns error", func() { @@ -289,7 +289,7 @@ func (suite *ExecutionResultInfoProviderSuite) TestExecutionResultQuery() { ) suite.Require().Error(err) suite.Require().Nil(query) - suite.Require().True(common.IsCriteriaNotMetError(err)) + suite.Require().True(optimistic_sync.IsCriteriaNotMetError(err)) }) } diff --git a/module/executiondatasync/optimistic_sync/execution_result_info_provider.go b/module/executiondatasync/optimistic_sync/execution_result_info_provider.go index 3d959d9a0bd..566fe97be3f 100644 --- a/module/executiondatasync/optimistic_sync/execution_result_info_provider.go +++ b/module/executiondatasync/optimistic_sync/execution_result_info_provider.go @@ -55,7 +55,7 @@ type ExecutionResultInfoProvider interface { // Expected error returns during normal operation: // - [common.InsufficientExecutionReceipts]: Found insufficient receipts for given block ID. // - [storage.ErrNotFound]: If the data was not found. - // - [common.RequiredExecutorsCountExceeded]: Required executor IDs count exceeds available executors. - // - [common.UnknownRequiredExecutor]: A required executor ID is not in the available set. + // - [optimistic_sync.RequiredExecutorsCountExceededError]: Required executor IDs count exceeds available executors. + // - [optimistic_sync.UnknownRequiredExecutorError]: A required executor ID is not in the available set. ExecutionResultInfo(blockID flow.Identifier, criteria Criteria) (*ExecutionResultInfo, error) } From 1382a76eb37d062693447b49dcca4fc1ebfaaa35 Mon Sep 17 00:00:00 2001 From: UlyanaAndrukhiv Date: Fri, 5 Dec 2025 15:42:34 +0200 Subject: [PATCH 05/15] Fixed info provider test --- .../optimistic_sync/execution_result/info_provider_test.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/module/executiondatasync/optimistic_sync/execution_result/info_provider_test.go b/module/executiondatasync/optimistic_sync/execution_result/info_provider_test.go index a513a66dfb9..df560f1e925 100644 --- a/module/executiondatasync/optimistic_sync/execution_result/info_provider_test.go +++ b/module/executiondatasync/optimistic_sync/execution_result/info_provider_test.go @@ -211,7 +211,7 @@ func (suite *ExecutionResultInfoProviderSuite) TestExecutionResultQuery() { receipts[i] = r } - suite.receipts.On("ByBlockID", block.ID()).Return(receipts, nil) + suite.receipts.On("ByBlockID", block.ID()).Return(receipts, nil).Once() suite.setupIdentitiesMock(allExecutionNodes) // Require executors that didn't produce any receipts From 6c9e292b81c6d3bc1225c241c6bee3dec2e3f3e8 Mon Sep 17 00:00:00 2001 From: UlyanaAndrukhiv Date: Fri, 5 Dec 2025 15:49:45 +0200 Subject: [PATCH 06/15] Small format and godoc refactoring --- module/executiondatasync/optimistic_sync/errors.go | 7 +++---- .../optimistic_sync/execution_result/info_provider.go | 3 +-- 2 files changed, 4 insertions(+), 6 deletions(-) diff --git a/module/executiondatasync/optimistic_sync/errors.go b/module/executiondatasync/optimistic_sync/errors.go index 1181b3ae202..dd154bbd543 100644 --- a/module/executiondatasync/optimistic_sync/errors.go +++ b/module/executiondatasync/optimistic_sync/errors.go @@ -22,15 +22,14 @@ func NewRequiredExecutorsCountExceededError(requiredExecutorsCount int, availabl } func (e *RequiredExecutorsCountExceededError) Error() string { - return fmt.Sprintf( - "required executors count exceeded: required %d, available %d", + return fmt.Sprintf("required executors count exceeded: required %d, available %d", e.requiredExecutorsCount, e.availableExecutorsCount, ) } func IsRequiredExecutorsCountExceededError(err error) bool { - var target *RequiredExecutorsCountExceededError - return errors.As(err, &target) + var requiredExecutorsCountExceededError *RequiredExecutorsCountExceededError + return errors.As(err, &requiredExecutorsCountExceededError) } // UnknownRequiredExecutorError indicates that a required executor ID is not present diff --git a/module/executiondatasync/optimistic_sync/execution_result/info_provider.go b/module/executiondatasync/optimistic_sync/execution_result/info_provider.go index 7ee0f2a377b..d1a10b8ec29 100644 --- a/module/executiondatasync/optimistic_sync/execution_result/info_provider.go +++ b/module/executiondatasync/optimistic_sync/execution_result/info_provider.go @@ -167,8 +167,7 @@ func (e *Provider) validateRequiredExecutors( // results are found, then the result with the most executors is returned. // // Expected errors during normal operations: -// - [common.MissingRequiredExecutor]: One or more required executors are not present. -// - [common.InsufficientExecutors]: The number of available executors is below the required minimum. +// - [common.InsufficientExecutionReceipts]: Found insufficient receipts for given block ID. func (e *Provider) findResultAndExecutors( blockID flow.Identifier, criteria optimistic_sync.Criteria, From 14919611c7c297f2007be090880ca6bdee834911 Mon Sep 17 00:00:00 2001 From: UlyanaAndrukhiv Date: Mon, 8 Dec 2025 12:29:32 +0200 Subject: [PATCH 07/15] Udded Unwrap method to optimistic_sync errors --- .../optimistic_sync/errors.go | 40 +++++++++++-------- 1 file changed, 24 insertions(+), 16 deletions(-) diff --git a/module/executiondatasync/optimistic_sync/errors.go b/module/executiondatasync/optimistic_sync/errors.go index dd154bbd543..82ca07233f9 100644 --- a/module/executiondatasync/optimistic_sync/errors.go +++ b/module/executiondatasync/optimistic_sync/errors.go @@ -10,21 +10,21 @@ import ( // RequiredExecutorsCountExceededError indicates that the requested number of required executors // exceeds the total available execution nodes. type RequiredExecutorsCountExceededError struct { - requiredExecutorsCount int - availableExecutorsCount int + err error } func NewRequiredExecutorsCountExceededError(requiredExecutorsCount int, availableExecutorsCount int) *RequiredExecutorsCountExceededError { return &RequiredExecutorsCountExceededError{ - requiredExecutorsCount: requiredExecutorsCount, - availableExecutorsCount: availableExecutorsCount, + err: fmt.Errorf("required executors count exceeded: required %d, available %d", requiredExecutorsCount, availableExecutorsCount), } } -func (e *RequiredExecutorsCountExceededError) Error() string { - return fmt.Sprintf("required executors count exceeded: required %d, available %d", - e.requiredExecutorsCount, e.availableExecutorsCount, - ) +func (e RequiredExecutorsCountExceededError) Error() string { + return e.err.Error() +} + +func (e RequiredExecutorsCountExceededError) Unwrap() error { + return e.err } func IsRequiredExecutorsCountExceededError(err error) bool { @@ -35,17 +35,21 @@ func IsRequiredExecutorsCountExceededError(err error) bool { // UnknownRequiredExecutorError indicates that a required executor ID is not present // in the list of active execution nodes. type UnknownRequiredExecutorError struct { - executorID flow.Identifier + err error } func NewUnknownRequiredExecutorError(executorID flow.Identifier) *UnknownRequiredExecutorError { return &UnknownRequiredExecutorError{ - executorID: executorID, + err: fmt.Errorf("unknown required executor ID %s", executorID.String()), } } -func (e *UnknownRequiredExecutorError) Error() string { - return fmt.Sprintf("unknown required executor ID: %s", e.executorID.String()) +func (e UnknownRequiredExecutorError) Error() string { + return e.err.Error() +} + +func (e UnknownRequiredExecutorError) Unwrap() error { + return e.err } func IsUnknownRequiredExecutorError(err error) bool { @@ -56,17 +60,21 @@ func IsUnknownRequiredExecutorError(err error) bool { // CriteriaNotMetError indicates that the execution result criteria could not be // satisfied for a given block, when the block is already sealed. type CriteriaNotMetError struct { - blockID flow.Identifier + err error } func NewCriteriaNotMetError(blockID flow.Identifier) *CriteriaNotMetError { return &CriteriaNotMetError{ - blockID: blockID, + err: fmt.Errorf("block %s is already sealed but the criteria is still not met", blockID), } } -func (e *CriteriaNotMetError) Error() string { - return fmt.Sprintf("block %s is already sealed but the criteria is still not met,", e.blockID) +func (e CriteriaNotMetError) Error() string { + return e.err.Error() +} + +func (e CriteriaNotMetError) Unwrap() error { + return e.err } func IsCriteriaNotMetError(err error) bool { From 79c056b97bff0268023a32a38f7afeb9185be845 Mon Sep 17 00:00:00 2001 From: UlyanaAndrukhiv Date: Mon, 8 Dec 2025 13:18:39 +0200 Subject: [PATCH 08/15] Updated execution result info impl for optimistic_sync by using all executions by block ID. Updated tests --- .../optimistic_sync/execution_result/info_provider.go | 2 +- .../optimistic_sync/execution_result/info_provider_test.go | 1 - 2 files changed, 1 insertion(+), 2 deletions(-) diff --git a/module/executiondatasync/optimistic_sync/execution_result/info_provider.go b/module/executiondatasync/optimistic_sync/execution_result/info_provider.go index d1a10b8ec29..ab237145c7e 100644 --- a/module/executiondatasync/optimistic_sync/execution_result/info_provider.go +++ b/module/executiondatasync/optimistic_sync/execution_result/info_provider.go @@ -61,7 +61,7 @@ func (e *Provider) ExecutionResultInfo( blockID flow.Identifier, criteria optimistic_sync.Criteria, ) (*optimistic_sync.ExecutionResultInfo, error) { - executorIdentities, err := e.state.Final().Identities(filter.HasRole[flow.Identity](flow.RoleExecution)) + executorIdentities, err := e.state.AtBlockID(blockID).Identities(filter.HasRole[flow.Identity](flow.RoleExecution)) if err != nil { return nil, fmt.Errorf("failed to retrieve execution IDs: %w", err) } diff --git a/module/executiondatasync/optimistic_sync/execution_result/info_provider_test.go b/module/executiondatasync/optimistic_sync/execution_result/info_provider_test.go index df560f1e925..836dd538339 100644 --- a/module/executiondatasync/optimistic_sync/execution_result/info_provider_test.go +++ b/module/executiondatasync/optimistic_sync/execution_result/info_provider_test.go @@ -52,7 +52,6 @@ func (suite *ExecutionResultInfoProviderSuite) SetupTest() { suite.state.On("SealedResult", rootBlockID).Return(flow.ExecutionReceiptList{}).Maybe() suite.params.On("SporkRootBlock").Return(suite.rootBlock) suite.state.On("Params").Return(suite.params) - suite.state.On("Final").Return(suite.snapshot, nil).Maybe() suite.state.On("AtBlockID", mock.Anything).Return(suite.snapshot).Maybe() } From 9addcea4077477cfc3acf2702df28a552e587fe7 Mon Sep 17 00:00:00 2001 From: UlyanaAndrukhiv Date: Mon, 8 Dec 2025 13:48:49 +0200 Subject: [PATCH 09/15] Added additional check for agreeing executors count. Updated error handling for fork-aware endpoints. Added test --- .../access/rpc/backend/accounts/accounts.go | 16 ++++++++ engine/access/rpc/backend/scripts/scripts.go | 6 +++ engine/access/state_stream/backend/backend.go | 2 + .../backend/backend_executiondata.go | 2 + .../optimistic_sync/errors.go | 25 ++++++++++++ .../execution_result/info_provider.go | 40 ++++++++++++------- .../execution_result/info_provider_test.go | 17 ++++++++ .../execution_result_info_provider.go | 1 + 8 files changed, 95 insertions(+), 14 deletions(-) diff --git a/engine/access/rpc/backend/accounts/accounts.go b/engine/access/rpc/backend/accounts/accounts.go index cc090b3d0b1..ffcd5c6aa3c 100644 --- a/engine/access/rpc/backend/accounts/accounts.go +++ b/engine/access/rpc/backend/accounts/accounts.go @@ -129,6 +129,8 @@ func (a *Accounts) GetAccountAtLatestBlock(ctx context.Context, address flow.Add return nil, nil, access.NewDataNotFoundError("execution data", err) case optimistic_sync.IsRequiredExecutorsCountExceededError(err): return nil, nil, access.NewInvalidRequestError(err) + case optimistic_sync.IsAgreeingExecutorsCountExceededError(err): + return nil, nil, access.NewInvalidRequestError(err) case optimistic_sync.IsUnknownRequiredExecutorError(err): return nil, nil, access.NewInvalidRequestError(err) case optimistic_sync.IsCriteriaNotMetError(err): @@ -189,6 +191,8 @@ func (a *Accounts) GetAccountAtBlockHeight( return nil, nil, access.NewDataNotFoundError("execution data", err) case optimistic_sync.IsRequiredExecutorsCountExceededError(err): return nil, nil, access.NewInvalidRequestError(err) + case optimistic_sync.IsAgreeingExecutorsCountExceededError(err): + return nil, nil, access.NewInvalidRequestError(err) case optimistic_sync.IsUnknownRequiredExecutorError(err): return nil, nil, access.NewInvalidRequestError(err) case optimistic_sync.IsCriteriaNotMetError(err): @@ -245,6 +249,8 @@ func (a *Accounts) GetAccountBalanceAtLatestBlock(ctx context.Context, address f return 0, nil, access.NewDataNotFoundError("execution data", err) case optimistic_sync.IsRequiredExecutorsCountExceededError(err): return 0, nil, access.NewInvalidRequestError(err) + case optimistic_sync.IsAgreeingExecutorsCountExceededError(err): + return 0, nil, access.NewInvalidRequestError(err) case optimistic_sync.IsUnknownRequiredExecutorError(err): return 0, nil, access.NewInvalidRequestError(err) case optimistic_sync.IsCriteriaNotMetError(err): @@ -305,6 +311,8 @@ func (a *Accounts) GetAccountBalanceAtBlockHeight( return 0, nil, access.NewDataNotFoundError("execution data", err) case optimistic_sync.IsRequiredExecutorsCountExceededError(err): return 0, nil, access.NewInvalidRequestError(err) + case optimistic_sync.IsAgreeingExecutorsCountExceededError(err): + return 0, nil, access.NewInvalidRequestError(err) case optimistic_sync.IsUnknownRequiredExecutorError(err): return 0, nil, access.NewInvalidRequestError(err) case optimistic_sync.IsCriteriaNotMetError(err): @@ -365,6 +373,8 @@ func (a *Accounts) GetAccountKeyAtLatestBlock( return nil, nil, access.NewDataNotFoundError("execution data", err) case optimistic_sync.IsRequiredExecutorsCountExceededError(err): return nil, nil, access.NewInvalidRequestError(err) + case optimistic_sync.IsAgreeingExecutorsCountExceededError(err): + return nil, nil, access.NewInvalidRequestError(err) case optimistic_sync.IsUnknownRequiredExecutorError(err): return nil, nil, access.NewInvalidRequestError(err) case optimistic_sync.IsCriteriaNotMetError(err): @@ -426,6 +436,8 @@ func (a *Accounts) GetAccountKeyAtBlockHeight( return nil, nil, access.NewDataNotFoundError("execution data", err) case optimistic_sync.IsRequiredExecutorsCountExceededError(err): return nil, nil, access.NewInvalidRequestError(err) + case optimistic_sync.IsAgreeingExecutorsCountExceededError(err): + return nil, nil, access.NewInvalidRequestError(err) case optimistic_sync.IsUnknownRequiredExecutorError(err): return nil, nil, access.NewInvalidRequestError(err) case optimistic_sync.IsCriteriaNotMetError(err): @@ -485,6 +497,8 @@ func (a *Accounts) GetAccountKeysAtLatestBlock( return nil, nil, access.NewDataNotFoundError("execution data", err) case optimistic_sync.IsRequiredExecutorsCountExceededError(err): return nil, nil, access.NewInvalidRequestError(err) + case optimistic_sync.IsAgreeingExecutorsCountExceededError(err): + return nil, nil, access.NewInvalidRequestError(err) case optimistic_sync.IsUnknownRequiredExecutorError(err): return nil, nil, access.NewInvalidRequestError(err) case optimistic_sync.IsCriteriaNotMetError(err): @@ -545,6 +559,8 @@ func (a *Accounts) GetAccountKeysAtBlockHeight( return nil, nil, access.NewDataNotFoundError("execution data", err) case optimistic_sync.IsRequiredExecutorsCountExceededError(err): return nil, nil, access.NewInvalidRequestError(err) + case optimistic_sync.IsAgreeingExecutorsCountExceededError(err): + return nil, nil, access.NewInvalidRequestError(err) case optimistic_sync.IsUnknownRequiredExecutorError(err): return nil, nil, access.NewInvalidRequestError(err) case optimistic_sync.IsCriteriaNotMetError(err): diff --git a/engine/access/rpc/backend/scripts/scripts.go b/engine/access/rpc/backend/scripts/scripts.go index e751ce19f73..a2f31f416a2 100644 --- a/engine/access/rpc/backend/scripts/scripts.go +++ b/engine/access/rpc/backend/scripts/scripts.go @@ -165,6 +165,8 @@ func (b *Scripts) ExecuteScriptAtLatestBlock( return nil, nil, access.NewDataNotFoundError("execution data", err) case optimistic_sync.IsRequiredExecutorsCountExceededError(err): return nil, nil, access.NewInvalidRequestError(err) + case optimistic_sync.IsAgreeingExecutorsCountExceededError(err): + return nil, nil, access.NewInvalidRequestError(err) case optimistic_sync.IsUnknownRequiredExecutorError(err): return nil, nil, access.NewInvalidRequestError(err) case optimistic_sync.IsCriteriaNotMetError(err): @@ -229,6 +231,8 @@ func (b *Scripts) ExecuteScriptAtBlockID( return nil, nil, access.NewDataNotFoundError("execution data", err) case optimistic_sync.IsRequiredExecutorsCountExceededError(err): return nil, nil, access.NewInvalidRequestError(err) + case optimistic_sync.IsAgreeingExecutorsCountExceededError(err): + return nil, nil, access.NewInvalidRequestError(err) case optimistic_sync.IsUnknownRequiredExecutorError(err): return nil, nil, access.NewInvalidRequestError(err) case optimistic_sync.IsCriteriaNotMetError(err): @@ -298,6 +302,8 @@ func (b *Scripts) ExecuteScriptAtBlockHeight( return nil, nil, access.NewDataNotFoundError("execution data", err) case optimistic_sync.IsRequiredExecutorsCountExceededError(err): return nil, nil, access.NewInvalidRequestError(err) + case optimistic_sync.IsAgreeingExecutorsCountExceededError(err): + return nil, nil, access.NewInvalidRequestError(err) case optimistic_sync.IsUnknownRequiredExecutorError(err): return nil, nil, access.NewInvalidRequestError(err) case optimistic_sync.IsCriteriaNotMetError(err): diff --git a/engine/access/state_stream/backend/backend.go b/engine/access/state_stream/backend/backend.go index c2ba6b9be8f..18180561d38 100644 --- a/engine/access/state_stream/backend/backend.go +++ b/engine/access/state_stream/backend/backend.go @@ -225,6 +225,8 @@ func (b *StateStreamBackend) GetRegisterValues( return nil, nil, access.NewDataNotFoundError("execution data", err) case optimistic_sync.IsRequiredExecutorsCountExceededError(err): return nil, nil, access.NewInvalidRequestError(err) + case optimistic_sync.IsAgreeingExecutorsCountExceededError(err): + return nil, nil, access.NewInvalidRequestError(err) case optimistic_sync.IsUnknownRequiredExecutorError(err): return nil, nil, access.NewInvalidRequestError(err) case optimistic_sync.IsCriteriaNotMetError(err): diff --git a/engine/access/state_stream/backend/backend_executiondata.go b/engine/access/state_stream/backend/backend_executiondata.go index 2ef38f48ebd..b631ed68eeb 100644 --- a/engine/access/state_stream/backend/backend_executiondata.go +++ b/engine/access/state_stream/backend/backend_executiondata.go @@ -63,6 +63,8 @@ func (b *ExecutionDataBackend) GetExecutionDataByBlockID( return nil, nil, access.NewDataNotFoundError("execution data", err) case optimistic_sync.IsRequiredExecutorsCountExceededError(err): return nil, nil, access.NewInvalidRequestError(err) + case optimistic_sync.IsAgreeingExecutorsCountExceededError(err): + return nil, nil, access.NewInvalidRequestError(err) case optimistic_sync.IsUnknownRequiredExecutorError(err): return nil, nil, access.NewInvalidRequestError(err) case optimistic_sync.IsCriteriaNotMetError(err): diff --git a/module/executiondatasync/optimistic_sync/errors.go b/module/executiondatasync/optimistic_sync/errors.go index 82ca07233f9..b4c5cdc3640 100644 --- a/module/executiondatasync/optimistic_sync/errors.go +++ b/module/executiondatasync/optimistic_sync/errors.go @@ -32,6 +32,31 @@ func IsRequiredExecutorsCountExceededError(err error) bool { return errors.As(err, &requiredExecutorsCountExceededError) } +// AgreeingExecutorsCountExceededError indicates that the requested number of agreeing executors +// exceeds the total available execution nodes. +type AgreeingExecutorsCountExceededError struct { + err error +} + +func NewAgreeingExecutorsCountExceededError(agreeingExecutorsCount uint, availableExecutorsCount int) *AgreeingExecutorsCountExceededError { + return &AgreeingExecutorsCountExceededError{ + err: fmt.Errorf("agreeing executors count exceeded: provided %d, available %d", agreeingExecutorsCount, availableExecutorsCount), + } +} + +func (e AgreeingExecutorsCountExceededError) Error() string { + return e.err.Error() +} + +func (e AgreeingExecutorsCountExceededError) Unwrap() error { + return e.err +} + +func IsAgreeingExecutorsCountExceededError(err error) bool { + var agreeingExecutorsCountExceededError *AgreeingExecutorsCountExceededError + return errors.As(err, &agreeingExecutorsCountExceededError) +} + // UnknownRequiredExecutorError indicates that a required executor ID is not present // in the list of active execution nodes. type UnknownRequiredExecutorError struct { diff --git a/module/executiondatasync/optimistic_sync/execution_result/info_provider.go b/module/executiondatasync/optimistic_sync/execution_result/info_provider.go index ab237145c7e..75f2e71505b 100644 --- a/module/executiondatasync/optimistic_sync/execution_result/info_provider.go +++ b/module/executiondatasync/optimistic_sync/execution_result/info_provider.go @@ -56,6 +56,7 @@ func NewExecutionResultInfoProvider( // - [common.InsufficientExecutionReceipts]: Found insufficient receipts for given block ID. // - [storage.ErrNotFound]: If the data was not found. // - [optimistic_sync.RequiredExecutorsCountExceededError]: Required executor IDs count exceeds available executors. +// - [optimistic_sync.AgreeingExecutorsCountExceededError]: Agreeing executors count exceeds available executors. // - [optimistic_sync.UnknownRequiredExecutorError]: A required executor ID is not in the available set. func (e *Provider) ExecutionResultInfo( blockID flow.Identifier, @@ -66,7 +67,7 @@ func (e *Provider) ExecutionResultInfo( return nil, fmt.Errorf("failed to retrieve execution IDs: %w", err) } - err = e.validateRequiredExecutors(criteria.RequiredExecutors, executorIdentities) + err = e.validateCriteria(criteria, executorIdentities) if err != nil { return nil, fmt.Errorf("invalid required executors: %w", err) } @@ -134,26 +135,37 @@ func (e *Provider) ExecutionResultInfo( }, nil } -// validateRequiredExecutors verifies that the provided set of execution node -// identities contains all nodes required for processing and that the requested -// number of required executors does not exceed the available number. +// validateCriteria verifies that the provided optimistic sync criteria can be +// satisfied by the currently available execution nodes. +// +// The validation ensures that the required executor IDs do not exceed the number +// of available executors, that the requested AgreeingExecutorsCount is feasible, +// and that every required executor ID is present in the available set. // // Expected errors during normal operations: -// - [common.RequiredExecutorsCountExceededError]: Required executor IDs count exceeds available executors. -// - [common.UnknownRequiredExecutorError]: A required executor ID is not in the available set. -func (e *Provider) validateRequiredExecutors( - required flow.IdentifierList, - available flow.IdentityList, +// - [optimistic_sync.RequiredExecutorsCountExceededError]: Required executor IDs count exceeds available executors. +// - [optimistic_sync.AgreeingExecutorsCountExceededError]: Agreeing executors count exceeds available executors. +// - [optimistic_sync.UnknownRequiredExecutorError]: A required executor ID is not in the available set. +func (e *Provider) validateCriteria( + criteria optimistic_sync.Criteria, + availableExecutors flow.IdentityList, ) error { - if len(available) < len(required) { + requiredExecutors := criteria.RequiredExecutors + if len(availableExecutors) < len(requiredExecutors) { return optimistic_sync.NewRequiredExecutorsCountExceededError( - len(required), - len(available), + len(requiredExecutors), + len(availableExecutors), + ) + } + if uint(len(availableExecutors)) < criteria.AgreeingExecutorsCount { + return optimistic_sync.NewAgreeingExecutorsCountExceededError( + criteria.AgreeingExecutorsCount, + len(availableExecutors), ) } - lookup := available.Lookup() - for _, executorID := range required { + lookup := availableExecutors.Lookup() + for _, executorID := range requiredExecutors { if _, ok := lookup[executorID]; !ok { return optimistic_sync.NewUnknownRequiredExecutorError(executorID) } diff --git a/module/executiondatasync/optimistic_sync/execution_result/info_provider_test.go b/module/executiondatasync/optimistic_sync/execution_result/info_provider_test.go index 836dd538339..c744d8ecc2f 100644 --- a/module/executiondatasync/optimistic_sync/execution_result/info_provider_test.go +++ b/module/executiondatasync/optimistic_sync/execution_result/info_provider_test.go @@ -243,6 +243,23 @@ func (suite *ExecutionResultInfoProviderSuite) TestExecutionResultQuery() { suite.Require().True(optimistic_sync.IsRequiredExecutorsCountExceededError(err)) }) + suite.Run("agreeing executors count is greater than available executors count returns error", func() { + provider := suite.createProvider(flow.IdentifierList{}, optimistic_sync.Criteria{}) + + suite.setupIdentitiesMock(allExecutionNodes) + requiredExecutors := allExecutionNodes.NodeIDs() + + query, err := provider.ExecutionResultInfo( + block.ID(), optimistic_sync.Criteria{ + AgreeingExecutorsCount: uint(len(allExecutionNodes) + 1), + RequiredExecutors: requiredExecutors, + }, + ) + suite.Require().Error(err) + suite.Require().Nil(query) + suite.Require().True(optimistic_sync.IsAgreeingExecutorsCountExceededError(err)) + }) + suite.Run("unknown required executor returns error", func() { provider := suite.createProvider(flow.IdentifierList{}, optimistic_sync.Criteria{}) diff --git a/module/executiondatasync/optimistic_sync/execution_result_info_provider.go b/module/executiondatasync/optimistic_sync/execution_result_info_provider.go index 566fe97be3f..403cb065701 100644 --- a/module/executiondatasync/optimistic_sync/execution_result_info_provider.go +++ b/module/executiondatasync/optimistic_sync/execution_result_info_provider.go @@ -56,6 +56,7 @@ type ExecutionResultInfoProvider interface { // - [common.InsufficientExecutionReceipts]: Found insufficient receipts for given block ID. // - [storage.ErrNotFound]: If the data was not found. // - [optimistic_sync.RequiredExecutorsCountExceededError]: Required executor IDs count exceeds available executors. + // - [optimistic_sync.AgreeingExecutorsCountExceededError]: Agreeing executors count exceeds available executors. // - [optimistic_sync.UnknownRequiredExecutorError]: A required executor ID is not in the available set. ExecutionResultInfo(blockID flow.Identifier, criteria Criteria) (*ExecutionResultInfo, error) } From 708500a6d6432312548fa8f77012c99620ec36a9 Mon Sep 17 00:00:00 2001 From: UlyanaAndrukhiv Date: Mon, 8 Dec 2025 18:14:11 +0200 Subject: [PATCH 10/15] Refactored CriteriaNotMetError, added BlockFinalityMismatchError according to suggestions. Added tests --- .../access/rpc/backend/accounts/accounts.go | 16 ++++++ engine/access/rpc/backend/scripts/scripts.go | 6 ++ engine/access/state_stream/backend/backend.go | 2 + .../backend/backend_executiondata.go | 2 + .../optimistic_sync/errors.go | 28 +++++++++- .../execution_result/info_provider.go | 26 +++++---- .../execution_result/info_provider_test.go | 55 +++++++++++++------ 7 files changed, 106 insertions(+), 29 deletions(-) diff --git a/engine/access/rpc/backend/accounts/accounts.go b/engine/access/rpc/backend/accounts/accounts.go index ffcd5c6aa3c..4915de75914 100644 --- a/engine/access/rpc/backend/accounts/accounts.go +++ b/engine/access/rpc/backend/accounts/accounts.go @@ -135,6 +135,8 @@ func (a *Accounts) GetAccountAtLatestBlock(ctx context.Context, address flow.Add return nil, nil, access.NewInvalidRequestError(err) case optimistic_sync.IsCriteriaNotMetError(err): return nil, nil, access.NewPreconditionFailedError(err) + case optimistic_sync.IsBlockFinalityMismatchError(err): + return nil, nil, access.NewInvalidRequestError(err) default: return nil, nil, access.RequireNoError(ctx, err) } @@ -197,6 +199,8 @@ func (a *Accounts) GetAccountAtBlockHeight( return nil, nil, access.NewInvalidRequestError(err) case optimistic_sync.IsCriteriaNotMetError(err): return nil, nil, access.NewPreconditionFailedError(err) + case optimistic_sync.IsBlockFinalityMismatchError(err): + return nil, nil, access.NewInvalidRequestError(err) default: return nil, nil, access.RequireNoError(ctx, err) } @@ -255,6 +259,8 @@ func (a *Accounts) GetAccountBalanceAtLatestBlock(ctx context.Context, address f return 0, nil, access.NewInvalidRequestError(err) case optimistic_sync.IsCriteriaNotMetError(err): return 0, nil, access.NewPreconditionFailedError(err) + case optimistic_sync.IsBlockFinalityMismatchError(err): + return 0, nil, access.NewInvalidRequestError(err) default: return 0, nil, access.RequireNoError(ctx, err) } @@ -317,6 +323,8 @@ func (a *Accounts) GetAccountBalanceAtBlockHeight( return 0, nil, access.NewInvalidRequestError(err) case optimistic_sync.IsCriteriaNotMetError(err): return 0, nil, access.NewPreconditionFailedError(err) + case optimistic_sync.IsBlockFinalityMismatchError(err): + return 0, nil, access.NewInvalidRequestError(err) default: return 0, nil, access.RequireNoError(ctx, err) } @@ -379,6 +387,8 @@ func (a *Accounts) GetAccountKeyAtLatestBlock( return nil, nil, access.NewInvalidRequestError(err) case optimistic_sync.IsCriteriaNotMetError(err): return nil, nil, access.NewPreconditionFailedError(err) + case optimistic_sync.IsBlockFinalityMismatchError(err): + return nil, nil, access.NewInvalidRequestError(err) default: return nil, nil, access.RequireNoError(ctx, err) } @@ -442,6 +452,8 @@ func (a *Accounts) GetAccountKeyAtBlockHeight( return nil, nil, access.NewInvalidRequestError(err) case optimistic_sync.IsCriteriaNotMetError(err): return nil, nil, access.NewPreconditionFailedError(err) + case optimistic_sync.IsBlockFinalityMismatchError(err): + return nil, nil, access.NewInvalidRequestError(err) default: return nil, nil, access.RequireNoError(ctx, err) } @@ -503,6 +515,8 @@ func (a *Accounts) GetAccountKeysAtLatestBlock( return nil, nil, access.NewInvalidRequestError(err) case optimistic_sync.IsCriteriaNotMetError(err): return nil, nil, access.NewPreconditionFailedError(err) + case optimistic_sync.IsBlockFinalityMismatchError(err): + return nil, nil, access.NewInvalidRequestError(err) default: return nil, nil, access.RequireNoError(ctx, err) } @@ -565,6 +579,8 @@ func (a *Accounts) GetAccountKeysAtBlockHeight( return nil, nil, access.NewInvalidRequestError(err) case optimistic_sync.IsCriteriaNotMetError(err): return nil, nil, access.NewPreconditionFailedError(err) + case optimistic_sync.IsBlockFinalityMismatchError(err): + return nil, nil, access.NewInvalidRequestError(err) default: return nil, nil, access.RequireNoError(ctx, err) } diff --git a/engine/access/rpc/backend/scripts/scripts.go b/engine/access/rpc/backend/scripts/scripts.go index a2f31f416a2..8d1cc0015cf 100644 --- a/engine/access/rpc/backend/scripts/scripts.go +++ b/engine/access/rpc/backend/scripts/scripts.go @@ -171,6 +171,8 @@ func (b *Scripts) ExecuteScriptAtLatestBlock( return nil, nil, access.NewInvalidRequestError(err) case optimistic_sync.IsCriteriaNotMetError(err): return nil, nil, access.NewPreconditionFailedError(err) + case optimistic_sync.IsBlockFinalityMismatchError(err): + return nil, nil, access.NewInvalidRequestError(err) default: return nil, nil, access.RequireNoError(ctx, err) } @@ -237,6 +239,8 @@ func (b *Scripts) ExecuteScriptAtBlockID( return nil, nil, access.NewInvalidRequestError(err) case optimistic_sync.IsCriteriaNotMetError(err): return nil, nil, access.NewPreconditionFailedError(err) + case optimistic_sync.IsBlockFinalityMismatchError(err): + return nil, nil, access.NewInvalidRequestError(err) default: return nil, nil, access.RequireNoError(ctx, err) } @@ -308,6 +312,8 @@ func (b *Scripts) ExecuteScriptAtBlockHeight( return nil, nil, access.NewInvalidRequestError(err) case optimistic_sync.IsCriteriaNotMetError(err): return nil, nil, access.NewPreconditionFailedError(err) + case optimistic_sync.IsBlockFinalityMismatchError(err): + return nil, nil, access.NewInvalidRequestError(err) default: return nil, nil, access.RequireNoError(ctx, err) } diff --git a/engine/access/state_stream/backend/backend.go b/engine/access/state_stream/backend/backend.go index 18180561d38..9c17db4d470 100644 --- a/engine/access/state_stream/backend/backend.go +++ b/engine/access/state_stream/backend/backend.go @@ -231,6 +231,8 @@ func (b *StateStreamBackend) GetRegisterValues( return nil, nil, access.NewInvalidRequestError(err) case optimistic_sync.IsCriteriaNotMetError(err): return nil, nil, access.NewPreconditionFailedError(err) + case optimistic_sync.IsBlockFinalityMismatchError(err): + return nil, nil, access.NewInvalidRequestError(err) default: return nil, nil, access.RequireNoError(ctx, err) } diff --git a/engine/access/state_stream/backend/backend_executiondata.go b/engine/access/state_stream/backend/backend_executiondata.go index b631ed68eeb..dd93baadd3a 100644 --- a/engine/access/state_stream/backend/backend_executiondata.go +++ b/engine/access/state_stream/backend/backend_executiondata.go @@ -69,6 +69,8 @@ func (b *ExecutionDataBackend) GetExecutionDataByBlockID( return nil, nil, access.NewInvalidRequestError(err) case optimistic_sync.IsCriteriaNotMetError(err): return nil, nil, access.NewPreconditionFailedError(err) + case optimistic_sync.IsBlockFinalityMismatchError(err): + return nil, nil, access.NewInvalidRequestError(err) default: return nil, nil, access.RequireNoError(ctx, err) } diff --git a/module/executiondatasync/optimistic_sync/errors.go b/module/executiondatasync/optimistic_sync/errors.go index b4c5cdc3640..3f6b3087880 100644 --- a/module/executiondatasync/optimistic_sync/errors.go +++ b/module/executiondatasync/optimistic_sync/errors.go @@ -90,7 +90,7 @@ type CriteriaNotMetError struct { func NewCriteriaNotMetError(blockID flow.Identifier) *CriteriaNotMetError { return &CriteriaNotMetError{ - err: fmt.Errorf("block %s is already sealed but the criteria is still not met", blockID), + err: fmt.Errorf("the criteria for block %s is not met", blockID), } } @@ -106,3 +106,29 @@ func IsCriteriaNotMetError(err error) bool { var criteriaNotMetError *CriteriaNotMetError return errors.As(err, &criteriaNotMetError) } + +// BlockFinalityMismatchError indicates that the requested block does not match +// the finalized block at the same height. This means the block cannot belong +// to the canonical finalized chain. +type BlockFinalityMismatchError struct { + err error +} + +func NewBlockFinalityMismatchError(blockID flow.Identifier, actualBlockID flow.Identifier) *BlockFinalityMismatchError { + return &BlockFinalityMismatchError{ + err: fmt.Errorf("block %s is not the finalized block at its height (finalized block is %s)", blockID, actualBlockID), + } +} + +func (e BlockFinalityMismatchError) Error() string { + return e.err.Error() +} + +func (e BlockFinalityMismatchError) Unwrap() error { + return e.err +} + +func IsBlockFinalityMismatchError(err error) bool { + var blockFinalityMismatchError *BlockFinalityMismatchError + return errors.As(err, &blockFinalityMismatchError) +} diff --git a/module/executiondatasync/optimistic_sync/execution_result/info_provider.go b/module/executiondatasync/optimistic_sync/execution_result/info_provider.go index 75f2e71505b..c2e17affd7a 100644 --- a/module/executiondatasync/optimistic_sync/execution_result/info_provider.go +++ b/module/executiondatasync/optimistic_sync/execution_result/info_provider.go @@ -104,16 +104,8 @@ func (e *Provider) ExecutionResultInfo( return nil, fmt.Errorf("failed to choose execution nodes for block ID %v: %w", blockID, err) } - sealedHeader, err := e.state.Sealed().Head() - if err != nil { - return nil, fmt.Errorf("failed to lookup sealed header: %w", err) - } - header, err := e.headers.ByBlockID(blockID) - if err != nil { - return nil, fmt.Errorf("failed to get header by block ID %v: %w", blockID, err) - } - // If block is sealed and criteria cannot be met return an error - if header.Height <= sealedHeader.Height && len(subsetENs) < len(criteria.RequiredExecutors) { + // If criteria cannot be met, return an error + if len(subsetENs) < len(criteria.RequiredExecutors) { return nil, optimistic_sync.NewCriteriaNotMetError(blockID) } @@ -129,6 +121,20 @@ func (e *Provider) ExecutionResultInfo( return nil, fmt.Errorf("no execution nodes found for result %v (blockID: %v): %w", result.ID(), blockID, err) } + header, err := e.headers.ByBlockID(blockID) + if err != nil { + return nil, fmt.Errorf("failed to get header by block ID %v: %w", blockID, err) + } + // Lookup the finalized block ID at the height of the requested block + blockIDFinalized, err := e.headers.BlockIDByHeight(header.Height) + if err != nil { + return nil, fmt.Errorf("failed to lookup finalized block ID at height %d: %w", header.Height, err) + } + // If the requested block conflicts with finalized block, return error + if blockIDFinalized != blockID { + return nil, optimistic_sync.NewBlockFinalityMismatchError(blockID, blockIDFinalized) + } + return &optimistic_sync.ExecutionResultInfo{ ExecutionResultID: result.ID(), ExecutionNodes: subsetENs, diff --git a/module/executiondatasync/optimistic_sync/execution_result/info_provider_test.go b/module/executiondatasync/optimistic_sync/execution_result/info_provider_test.go index c744d8ecc2f..2466b4c2c1e 100644 --- a/module/executiondatasync/optimistic_sync/execution_result/info_provider_test.go +++ b/module/executiondatasync/optimistic_sync/execution_result/info_provider_test.go @@ -107,8 +107,7 @@ func (suite *ExecutionResultInfoProviderSuite) TestExecutionResultQuery() { suite.receipts.On("ByBlockID", block.ID()).Return(receipts, nil).Once() suite.headers.On("ByBlockID", block.ID()).Return(block.ToHeader(), nil).Once() - suite.state.On("Sealed").Return(suite.snapshot, nil).Once() - suite.snapshot.On("Head").Return(func() *flow.Header { return block.ToHeader() }, nil).Once() + suite.headers.On("BlockIDByHeight", block.Height).Return(block.ID(), nil).Once() suite.setupIdentitiesMock(allExecutionNodes) // Require specific executors (first two nodes) @@ -155,8 +154,7 @@ func (suite *ExecutionResultInfoProviderSuite) TestExecutionResultQuery() { suite.setupIdentitiesMock(allExecutionNodes) suite.receipts.On("ByBlockID", block.ID()).Return(receipts, nil).Once() suite.headers.On("ByBlockID", block.ID()).Return(block.ToHeader(), nil).Once() - suite.state.On("Sealed").Return(suite.snapshot, nil).Once() - suite.snapshot.On("Head").Return(func() *flow.Header { return block.ToHeader() }, nil).Once() + suite.headers.On("BlockIDByHeight", block.Height).Return(block.ID(), nil).Once() query, err := provider.ExecutionResultInfo(block.ID(), optimistic_sync.Criteria{}) suite.Require().NoError(err) @@ -280,7 +278,7 @@ func (suite *ExecutionResultInfoProviderSuite) TestExecutionResultQuery() { suite.Require().True(optimistic_sync.IsUnknownRequiredExecutorError(err)) }) - suite.Run("criteria not met on sealed block returns error", func() { + suite.Run("criteria not met returns error", func() { provider := suite.createProvider(flow.IdentifierList{}, optimistic_sync.Criteria{}) receipts := make(flow.ExecutionReceiptList, totalReceipts) @@ -291,9 +289,6 @@ func (suite *ExecutionResultInfoProviderSuite) TestExecutionResultQuery() { receipts[i] = r } suite.receipts.On("ByBlockID", block.ID()).Return(receipts, nil).Once() - suite.headers.On("ByBlockID", block.ID()).Return(block.ToHeader(), nil).Once() - suite.state.On("Sealed").Return(suite.snapshot, nil).Once() - suite.snapshot.On("Head").Return(func() *flow.Header { return block.ToHeader() }, nil).Once() suite.setupIdentitiesMock(allExecutionNodes) // Require all executors, but only one produces receipts @@ -307,6 +302,35 @@ func (suite *ExecutionResultInfoProviderSuite) TestExecutionResultQuery() { suite.Require().Nil(query) suite.Require().True(optimistic_sync.IsCriteriaNotMetError(err)) }) + + suite.Run("requested block conflicts with a finalized, returns error", func() { + provider := suite.createProvider(flow.IdentifierList{}, optimistic_sync.Criteria{}) + + receipts := make(flow.ExecutionReceiptList, totalReceipts) + for i := 0; i < totalReceipts; i++ { + r := unittest.ReceiptForBlockFixture(block) + r.ExecutorID = allExecutionNodes[i].NodeID + r.ExecutionResult = *executionResult + receipts[i] = r + } + suite.receipts.On("ByBlockID", block.ID()).Return(receipts, nil).Once() + suite.headers.On("ByBlockID", block.ID()).Return(block.ToHeader(), nil).Once() + finalizedBlockID := unittest.IdentifierFixture() + suite.headers.On("BlockIDByHeight", block.Height).Return(finalizedBlockID, nil).Once() + suite.setupIdentitiesMock(allExecutionNodes) + + // Require all executors, but only one produces receipts + query, err := provider.ExecutionResultInfo( + block.ID(), optimistic_sync.Criteria{ + AgreeingExecutorsCount: 1, + RequiredExecutors: allExecutionNodes.NodeIDs(), + }, + ) + suite.Require().Error(err) + suite.Require().Nil(query) + suite.Require().True(optimistic_sync.IsBlockFinalityMismatchError(err)) + }) + } // TestRootBlockHandling tests the special case handling for root blocks. @@ -371,8 +395,7 @@ func (suite *ExecutionResultInfoProviderSuite) TestPreferredAndRequiredExecution suite.Run( "with default optimistic_sync.Criteria", func() { suite.headers.On("ByBlockID", block.ID()).Return(block.ToHeader(), nil).Once() - suite.state.On("Sealed").Return(suite.snapshot, nil).Once() - suite.snapshot.On("Head").Return(func() *flow.Header { return block.ToHeader() }, nil).Once() + suite.headers.On("BlockIDByHeight", block.Height).Return(block.ID(), nil).Once() suite.setupIdentitiesMock(allExecutionNodes) provider := suite.createProvider(flow.IdentifierList{}, optimistic_sync.Criteria{}) @@ -392,8 +415,7 @@ func (suite *ExecutionResultInfoProviderSuite) TestPreferredAndRequiredExecution suite.Run( "with operator preferred executors", func() { suite.headers.On("ByBlockID", block.ID()).Return(block.ToHeader(), nil).Once() - suite.state.On("Sealed").Return(suite.snapshot, nil).Once() - suite.snapshot.On("Head").Return(func() *flow.Header { return block.ToHeader() }, nil).Once() + suite.headers.On("BlockIDByHeight", block.Height).Return(block.ID(), nil).Once() suite.setupIdentitiesMock(allExecutionNodes) provider := suite.createProvider( @@ -417,8 +439,7 @@ func (suite *ExecutionResultInfoProviderSuite) TestPreferredAndRequiredExecution suite.Run( "with operator required executors", func() { suite.headers.On("ByBlockID", block.ID()).Return(block.ToHeader(), nil).Once() - suite.state.On("Sealed").Return(suite.snapshot, nil).Once() - suite.snapshot.On("Head").Return(func() *flow.Header { return block.ToHeader() }, nil).Once() + suite.headers.On("BlockIDByHeight", block.Height).Return(block.ID(), nil).Once() suite.setupIdentitiesMock(allExecutionNodes) provider := suite.createProvider( @@ -443,8 +464,7 @@ func (suite *ExecutionResultInfoProviderSuite) TestPreferredAndRequiredExecution suite.Run( "with both: operator preferred & required executors", func() { suite.headers.On("ByBlockID", block.ID()).Return(block.ToHeader(), nil).Once() - suite.state.On("Sealed").Return(suite.snapshot, nil).Once() - suite.snapshot.On("Head").Return(func() *flow.Header { return block.ToHeader() }, nil).Once() + suite.headers.On("BlockIDByHeight", block.Height).Return(block.ID(), nil).Once() suite.setupIdentitiesMock(allExecutionNodes) provider := suite.createProvider( @@ -472,8 +492,7 @@ func (suite *ExecutionResultInfoProviderSuite) TestPreferredAndRequiredExecution suite.Run( "with client preferred executors", func() { suite.headers.On("ByBlockID", block.ID()).Return(block.ToHeader(), nil).Once() - suite.state.On("Sealed").Return(suite.snapshot, nil).Once() - suite.snapshot.On("Head").Return(func() *flow.Header { return block.ToHeader() }, nil).Once() + suite.headers.On("BlockIDByHeight", block.Height).Return(block.ID(), nil).Once() suite.setupIdentitiesMock(allExecutionNodes) provider := suite.createProvider( From ed4f7d03a9e2726b05b0e623f45793344aca34cb Mon Sep 17 00:00:00 2001 From: UlyanaAndrukhiv Date: Mon, 8 Dec 2025 18:37:02 +0200 Subject: [PATCH 11/15] Updated godocs --- .../optimistic_sync/execution_result/info_provider.go | 4 ++++ .../optimistic_sync/execution_result_info_provider.go | 4 ++++ 2 files changed, 8 insertions(+) diff --git a/module/executiondatasync/optimistic_sync/execution_result/info_provider.go b/module/executiondatasync/optimistic_sync/execution_result/info_provider.go index c2e17affd7a..c61def919b4 100644 --- a/module/executiondatasync/optimistic_sync/execution_result/info_provider.go +++ b/module/executiondatasync/optimistic_sync/execution_result/info_provider.go @@ -58,6 +58,10 @@ func NewExecutionResultInfoProvider( // - [optimistic_sync.RequiredExecutorsCountExceededError]: Required executor IDs count exceeds available executors. // - [optimistic_sync.AgreeingExecutorsCountExceededError]: Agreeing executors count exceeds available executors. // - [optimistic_sync.UnknownRequiredExecutorError]: A required executor ID is not in the available set. +// - [optimistic_sync.CriteriaNotMetError]: Returned when the execution result +// criteria cannot be satisfied for the requested block. +// - [optimistic_sync.BlockFinalityMismatchError]: Returned when the requested +// block does not match the canonical finalized block at its height. func (e *Provider) ExecutionResultInfo( blockID flow.Identifier, criteria optimistic_sync.Criteria, diff --git a/module/executiondatasync/optimistic_sync/execution_result_info_provider.go b/module/executiondatasync/optimistic_sync/execution_result_info_provider.go index 403cb065701..ad426ac5c57 100644 --- a/module/executiondatasync/optimistic_sync/execution_result_info_provider.go +++ b/module/executiondatasync/optimistic_sync/execution_result_info_provider.go @@ -58,5 +58,9 @@ type ExecutionResultInfoProvider interface { // - [optimistic_sync.RequiredExecutorsCountExceededError]: Required executor IDs count exceeds available executors. // - [optimistic_sync.AgreeingExecutorsCountExceededError]: Agreeing executors count exceeds available executors. // - [optimistic_sync.UnknownRequiredExecutorError]: A required executor ID is not in the available set. + // - [optimistic_sync.CriteriaNotMetError]: Returned when the execution result + // criteria cannot be satisfied for the requested block. + // - [optimistic_sync.BlockFinalityMismatchError]: Returned when the requested + // block does not match the canonical finalized block at its height. ExecutionResultInfo(blockID flow.Identifier, criteria Criteria) (*ExecutionResultInfo, error) } From c26a53e94bb2aeae179b801e555942644b8781d3 Mon Sep 17 00:00:00 2001 From: UlyanaAndrukhiv Date: Wed, 10 Dec 2025 15:46:12 +0200 Subject: [PATCH 12/15] Refactored check for Criteria not met, updated tests --- .../access/rpc/backend/accounts/accounts.go | 16 ---- engine/access/rpc/backend/scripts/scripts.go | 6 -- engine/access/state_stream/backend/backend.go | 2 - .../backend/backend_executiondata.go | 2 - .../optimistic_sync/errors.go | 80 +++++-------------- .../execution_result/info_provider.go | 65 +++++++++------ .../execution_result_info_provider.go | 6 +- 7 files changed, 63 insertions(+), 114 deletions(-) diff --git a/engine/access/rpc/backend/accounts/accounts.go b/engine/access/rpc/backend/accounts/accounts.go index 4915de75914..2015896dc86 100644 --- a/engine/access/rpc/backend/accounts/accounts.go +++ b/engine/access/rpc/backend/accounts/accounts.go @@ -134,8 +134,6 @@ func (a *Accounts) GetAccountAtLatestBlock(ctx context.Context, address flow.Add case optimistic_sync.IsUnknownRequiredExecutorError(err): return nil, nil, access.NewInvalidRequestError(err) case optimistic_sync.IsCriteriaNotMetError(err): - return nil, nil, access.NewPreconditionFailedError(err) - case optimistic_sync.IsBlockFinalityMismatchError(err): return nil, nil, access.NewInvalidRequestError(err) default: return nil, nil, access.RequireNoError(ctx, err) @@ -198,8 +196,6 @@ func (a *Accounts) GetAccountAtBlockHeight( case optimistic_sync.IsUnknownRequiredExecutorError(err): return nil, nil, access.NewInvalidRequestError(err) case optimistic_sync.IsCriteriaNotMetError(err): - return nil, nil, access.NewPreconditionFailedError(err) - case optimistic_sync.IsBlockFinalityMismatchError(err): return nil, nil, access.NewInvalidRequestError(err) default: return nil, nil, access.RequireNoError(ctx, err) @@ -258,8 +254,6 @@ func (a *Accounts) GetAccountBalanceAtLatestBlock(ctx context.Context, address f case optimistic_sync.IsUnknownRequiredExecutorError(err): return 0, nil, access.NewInvalidRequestError(err) case optimistic_sync.IsCriteriaNotMetError(err): - return 0, nil, access.NewPreconditionFailedError(err) - case optimistic_sync.IsBlockFinalityMismatchError(err): return 0, nil, access.NewInvalidRequestError(err) default: return 0, nil, access.RequireNoError(ctx, err) @@ -322,8 +316,6 @@ func (a *Accounts) GetAccountBalanceAtBlockHeight( case optimistic_sync.IsUnknownRequiredExecutorError(err): return 0, nil, access.NewInvalidRequestError(err) case optimistic_sync.IsCriteriaNotMetError(err): - return 0, nil, access.NewPreconditionFailedError(err) - case optimistic_sync.IsBlockFinalityMismatchError(err): return 0, nil, access.NewInvalidRequestError(err) default: return 0, nil, access.RequireNoError(ctx, err) @@ -386,8 +378,6 @@ func (a *Accounts) GetAccountKeyAtLatestBlock( case optimistic_sync.IsUnknownRequiredExecutorError(err): return nil, nil, access.NewInvalidRequestError(err) case optimistic_sync.IsCriteriaNotMetError(err): - return nil, nil, access.NewPreconditionFailedError(err) - case optimistic_sync.IsBlockFinalityMismatchError(err): return nil, nil, access.NewInvalidRequestError(err) default: return nil, nil, access.RequireNoError(ctx, err) @@ -451,8 +441,6 @@ func (a *Accounts) GetAccountKeyAtBlockHeight( case optimistic_sync.IsUnknownRequiredExecutorError(err): return nil, nil, access.NewInvalidRequestError(err) case optimistic_sync.IsCriteriaNotMetError(err): - return nil, nil, access.NewPreconditionFailedError(err) - case optimistic_sync.IsBlockFinalityMismatchError(err): return nil, nil, access.NewInvalidRequestError(err) default: return nil, nil, access.RequireNoError(ctx, err) @@ -514,8 +502,6 @@ func (a *Accounts) GetAccountKeysAtLatestBlock( case optimistic_sync.IsUnknownRequiredExecutorError(err): return nil, nil, access.NewInvalidRequestError(err) case optimistic_sync.IsCriteriaNotMetError(err): - return nil, nil, access.NewPreconditionFailedError(err) - case optimistic_sync.IsBlockFinalityMismatchError(err): return nil, nil, access.NewInvalidRequestError(err) default: return nil, nil, access.RequireNoError(ctx, err) @@ -578,8 +564,6 @@ func (a *Accounts) GetAccountKeysAtBlockHeight( case optimistic_sync.IsUnknownRequiredExecutorError(err): return nil, nil, access.NewInvalidRequestError(err) case optimistic_sync.IsCriteriaNotMetError(err): - return nil, nil, access.NewPreconditionFailedError(err) - case optimistic_sync.IsBlockFinalityMismatchError(err): return nil, nil, access.NewInvalidRequestError(err) default: return nil, nil, access.RequireNoError(ctx, err) diff --git a/engine/access/rpc/backend/scripts/scripts.go b/engine/access/rpc/backend/scripts/scripts.go index 8d1cc0015cf..17e39a65a7c 100644 --- a/engine/access/rpc/backend/scripts/scripts.go +++ b/engine/access/rpc/backend/scripts/scripts.go @@ -170,8 +170,6 @@ func (b *Scripts) ExecuteScriptAtLatestBlock( case optimistic_sync.IsUnknownRequiredExecutorError(err): return nil, nil, access.NewInvalidRequestError(err) case optimistic_sync.IsCriteriaNotMetError(err): - return nil, nil, access.NewPreconditionFailedError(err) - case optimistic_sync.IsBlockFinalityMismatchError(err): return nil, nil, access.NewInvalidRequestError(err) default: return nil, nil, access.RequireNoError(ctx, err) @@ -238,8 +236,6 @@ func (b *Scripts) ExecuteScriptAtBlockID( case optimistic_sync.IsUnknownRequiredExecutorError(err): return nil, nil, access.NewInvalidRequestError(err) case optimistic_sync.IsCriteriaNotMetError(err): - return nil, nil, access.NewPreconditionFailedError(err) - case optimistic_sync.IsBlockFinalityMismatchError(err): return nil, nil, access.NewInvalidRequestError(err) default: return nil, nil, access.RequireNoError(ctx, err) @@ -311,8 +307,6 @@ func (b *Scripts) ExecuteScriptAtBlockHeight( case optimistic_sync.IsUnknownRequiredExecutorError(err): return nil, nil, access.NewInvalidRequestError(err) case optimistic_sync.IsCriteriaNotMetError(err): - return nil, nil, access.NewPreconditionFailedError(err) - case optimistic_sync.IsBlockFinalityMismatchError(err): return nil, nil, access.NewInvalidRequestError(err) default: return nil, nil, access.RequireNoError(ctx, err) diff --git a/engine/access/state_stream/backend/backend.go b/engine/access/state_stream/backend/backend.go index 9c17db4d470..6574c4d8ac4 100644 --- a/engine/access/state_stream/backend/backend.go +++ b/engine/access/state_stream/backend/backend.go @@ -230,8 +230,6 @@ func (b *StateStreamBackend) GetRegisterValues( case optimistic_sync.IsUnknownRequiredExecutorError(err): return nil, nil, access.NewInvalidRequestError(err) case optimistic_sync.IsCriteriaNotMetError(err): - return nil, nil, access.NewPreconditionFailedError(err) - case optimistic_sync.IsBlockFinalityMismatchError(err): return nil, nil, access.NewInvalidRequestError(err) default: return nil, nil, access.RequireNoError(ctx, err) diff --git a/engine/access/state_stream/backend/backend_executiondata.go b/engine/access/state_stream/backend/backend_executiondata.go index dd93baadd3a..89b13f0cddc 100644 --- a/engine/access/state_stream/backend/backend_executiondata.go +++ b/engine/access/state_stream/backend/backend_executiondata.go @@ -68,8 +68,6 @@ func (b *ExecutionDataBackend) GetExecutionDataByBlockID( case optimistic_sync.IsUnknownRequiredExecutorError(err): return nil, nil, access.NewInvalidRequestError(err) case optimistic_sync.IsCriteriaNotMetError(err): - return nil, nil, access.NewPreconditionFailedError(err) - case optimistic_sync.IsBlockFinalityMismatchError(err): return nil, nil, access.NewInvalidRequestError(err) default: return nil, nil, access.RequireNoError(ctx, err) diff --git a/module/executiondatasync/optimistic_sync/errors.go b/module/executiondatasync/optimistic_sync/errors.go index 3f6b3087880..8ddd3a05919 100644 --- a/module/executiondatasync/optimistic_sync/errors.go +++ b/module/executiondatasync/optimistic_sync/errors.go @@ -10,21 +10,21 @@ import ( // RequiredExecutorsCountExceededError indicates that the requested number of required executors // exceeds the total available execution nodes. type RequiredExecutorsCountExceededError struct { - err error + requiredExecutorsCount int + availableExecutorsCount int } func NewRequiredExecutorsCountExceededError(requiredExecutorsCount int, availableExecutorsCount int) *RequiredExecutorsCountExceededError { return &RequiredExecutorsCountExceededError{ - err: fmt.Errorf("required executors count exceeded: required %d, available %d", requiredExecutorsCount, availableExecutorsCount), + requiredExecutorsCount: requiredExecutorsCount, + availableExecutorsCount: availableExecutorsCount, } } -func (e RequiredExecutorsCountExceededError) Error() string { - return e.err.Error() -} - -func (e RequiredExecutorsCountExceededError) Unwrap() error { - return e.err +func (e *RequiredExecutorsCountExceededError) Error() string { + return fmt.Sprintf("required executors count exceeded: required %d, available %d", + e.requiredExecutorsCount, e.availableExecutorsCount, + ) } func IsRequiredExecutorsCountExceededError(err error) bool { @@ -35,21 +35,19 @@ func IsRequiredExecutorsCountExceededError(err error) bool { // AgreeingExecutorsCountExceededError indicates that the requested number of agreeing executors // exceeds the total available execution nodes. type AgreeingExecutorsCountExceededError struct { - err error + agreeingExecutorsCount uint + availableExecutorsCount int } func NewAgreeingExecutorsCountExceededError(agreeingExecutorsCount uint, availableExecutorsCount int) *AgreeingExecutorsCountExceededError { return &AgreeingExecutorsCountExceededError{ - err: fmt.Errorf("agreeing executors count exceeded: provided %d, available %d", agreeingExecutorsCount, availableExecutorsCount), + agreeingExecutorsCount: agreeingExecutorsCount, + availableExecutorsCount: availableExecutorsCount, } } -func (e AgreeingExecutorsCountExceededError) Error() string { - return e.err.Error() -} - -func (e AgreeingExecutorsCountExceededError) Unwrap() error { - return e.err +func (e *AgreeingExecutorsCountExceededError) Error() string { + return fmt.Sprintf("agreeing executors count exceeded: provided %d, available %d", e.agreeingExecutorsCount, e.availableExecutorsCount) } func IsAgreeingExecutorsCountExceededError(err error) bool { @@ -60,21 +58,17 @@ func IsAgreeingExecutorsCountExceededError(err error) bool { // UnknownRequiredExecutorError indicates that a required executor ID is not present // in the list of active execution nodes. type UnknownRequiredExecutorError struct { - err error + executorID flow.Identifier } func NewUnknownRequiredExecutorError(executorID flow.Identifier) *UnknownRequiredExecutorError { return &UnknownRequiredExecutorError{ - err: fmt.Errorf("unknown required executor ID %s", executorID.String()), + executorID: executorID, } } -func (e UnknownRequiredExecutorError) Error() string { - return e.err.Error() -} - -func (e UnknownRequiredExecutorError) Unwrap() error { - return e.err +func (e *UnknownRequiredExecutorError) Error() string { + return fmt.Sprintf("unknown required executor ID: %s", e.executorID.String()) } func IsUnknownRequiredExecutorError(err error) bool { @@ -85,50 +79,20 @@ func IsUnknownRequiredExecutorError(err error) bool { // CriteriaNotMetError indicates that the execution result criteria could not be // satisfied for a given block, when the block is already sealed. type CriteriaNotMetError struct { - err error + blockID flow.Identifier } func NewCriteriaNotMetError(blockID flow.Identifier) *CriteriaNotMetError { return &CriteriaNotMetError{ - err: fmt.Errorf("the criteria for block %s is not met", blockID), + blockID: blockID, } } -func (e CriteriaNotMetError) Error() string { - return e.err.Error() -} - -func (e CriteriaNotMetError) Unwrap() error { - return e.err +func (e *CriteriaNotMetError) Error() string { + return fmt.Sprintf("block %s is already sealed and no execution result satisfies the criteria", e.blockID) } func IsCriteriaNotMetError(err error) bool { var criteriaNotMetError *CriteriaNotMetError return errors.As(err, &criteriaNotMetError) } - -// BlockFinalityMismatchError indicates that the requested block does not match -// the finalized block at the same height. This means the block cannot belong -// to the canonical finalized chain. -type BlockFinalityMismatchError struct { - err error -} - -func NewBlockFinalityMismatchError(blockID flow.Identifier, actualBlockID flow.Identifier) *BlockFinalityMismatchError { - return &BlockFinalityMismatchError{ - err: fmt.Errorf("block %s is not the finalized block at its height (finalized block is %s)", blockID, actualBlockID), - } -} - -func (e BlockFinalityMismatchError) Error() string { - return e.err.Error() -} - -func (e BlockFinalityMismatchError) Unwrap() error { - return e.err -} - -func IsBlockFinalityMismatchError(err error) bool { - var blockFinalityMismatchError *BlockFinalityMismatchError - return errors.As(err, &blockFinalityMismatchError) -} diff --git a/module/executiondatasync/optimistic_sync/execution_result/info_provider.go b/module/executiondatasync/optimistic_sync/execution_result/info_provider.go index c61def919b4..558d8a56844 100644 --- a/module/executiondatasync/optimistic_sync/execution_result/info_provider.go +++ b/module/executiondatasync/optimistic_sync/execution_result/info_provider.go @@ -58,10 +58,8 @@ func NewExecutionResultInfoProvider( // - [optimistic_sync.RequiredExecutorsCountExceededError]: Required executor IDs count exceeds available executors. // - [optimistic_sync.AgreeingExecutorsCountExceededError]: Agreeing executors count exceeds available executors. // - [optimistic_sync.UnknownRequiredExecutorError]: A required executor ID is not in the available set. -// - [optimistic_sync.CriteriaNotMetError]: Returned when the execution result -// criteria cannot be satisfied for the requested block. -// - [optimistic_sync.BlockFinalityMismatchError]: Returned when the requested -// block does not match the canonical finalized block at its height. +// - [optimistic_sync.CriteriaNotMetError]: Returned when the block is already +// sealed but no execution result can satisfy the provided criteria. func (e *Provider) ExecutionResultInfo( blockID flow.Identifier, criteria optimistic_sync.Criteria, @@ -97,9 +95,43 @@ func (e *Provider) ExecutionResultInfo( }, nil } - result, executorIDs, err := e.findResultAndExecutors(blockID, criteria) - if err != nil { - return nil, fmt.Errorf("failed to find result and executors for block ID %v: %w", blockID, err) + result, executorIDs, findResultErr := e.findResultAndExecutors(blockID, criteria) + if findResultErr != nil { + findResultErr = fmt.Errorf("failed to find result and executors for block ID %v: %w", blockID, findResultErr) + // We want to return a more specific error when no matching execution results were found. + // This helps callers understand that their criteria likely cannot be met — especially if the + // block is already sealed. + + // Step 1: Get the block header + header, err := e.headers.ByBlockID(blockID) + if err != nil { + return nil, fmt.Errorf("failed to get header by block ID %v: %w", blockID, findResultErr) + } + + // Step 2a: Lookup the finalized block ID at this height + blockIDFinalized, err := e.headers.BlockIDByHeight(header.Height) + if err != nil { + return nil, fmt.Errorf("failed to lookup finalized block ID at height %d: %w", header.Height, findResultErr) + } + + // Step 2b: Check if this block is finalized. + // If BlockIDByHeight returns an ID that doesn't match, not finalized. + isFinalized := blockIDFinalized == blockID + + // Step 3: Check sealed status only if block finalized. + if isFinalized { + sealedHeader, err := e.state.Sealed().Head() + if err != nil { + return nil, fmt.Errorf("failed to lookup sealed header: %w", err) + } + + // If block is sealed, and didn't find any matching results return an error + if header.Height <= sealedHeader.Height { + return nil, optimistic_sync.NewCriteriaNotMetError(blockID) + } + } + + return nil, findResultErr } executors := executorIdentities.Filter(filter.HasNodeID[flow.Identity](executorIDs...)) @@ -108,11 +140,6 @@ func (e *Provider) ExecutionResultInfo( return nil, fmt.Errorf("failed to choose execution nodes for block ID %v: %w", blockID, err) } - // If criteria cannot be met, return an error - if len(subsetENs) < len(criteria.RequiredExecutors) { - return nil, optimistic_sync.NewCriteriaNotMetError(blockID) - } - if len(subsetENs) == 0 { // this is unexpected, and probably indicates there is a bug. // There are only three ways that SelectExecutionNodes can return an empty list: @@ -125,20 +152,6 @@ func (e *Provider) ExecutionResultInfo( return nil, fmt.Errorf("no execution nodes found for result %v (blockID: %v): %w", result.ID(), blockID, err) } - header, err := e.headers.ByBlockID(blockID) - if err != nil { - return nil, fmt.Errorf("failed to get header by block ID %v: %w", blockID, err) - } - // Lookup the finalized block ID at the height of the requested block - blockIDFinalized, err := e.headers.BlockIDByHeight(header.Height) - if err != nil { - return nil, fmt.Errorf("failed to lookup finalized block ID at height %d: %w", header.Height, err) - } - // If the requested block conflicts with finalized block, return error - if blockIDFinalized != blockID { - return nil, optimistic_sync.NewBlockFinalityMismatchError(blockID, blockIDFinalized) - } - return &optimistic_sync.ExecutionResultInfo{ ExecutionResultID: result.ID(), ExecutionNodes: subsetENs, diff --git a/module/executiondatasync/optimistic_sync/execution_result_info_provider.go b/module/executiondatasync/optimistic_sync/execution_result_info_provider.go index ad426ac5c57..868fd5e6553 100644 --- a/module/executiondatasync/optimistic_sync/execution_result_info_provider.go +++ b/module/executiondatasync/optimistic_sync/execution_result_info_provider.go @@ -58,9 +58,7 @@ type ExecutionResultInfoProvider interface { // - [optimistic_sync.RequiredExecutorsCountExceededError]: Required executor IDs count exceeds available executors. // - [optimistic_sync.AgreeingExecutorsCountExceededError]: Agreeing executors count exceeds available executors. // - [optimistic_sync.UnknownRequiredExecutorError]: A required executor ID is not in the available set. - // - [optimistic_sync.CriteriaNotMetError]: Returned when the execution result - // criteria cannot be satisfied for the requested block. - // - [optimistic_sync.BlockFinalityMismatchError]: Returned when the requested - // block does not match the canonical finalized block at its height. + // - [optimistic_sync.CriteriaNotMetError]: Returned when the block is already + // sealed but no execution result can satisfy the provided criteria. ExecutionResultInfo(blockID flow.Identifier, criteria Criteria) (*ExecutionResultInfo, error) } From 440903ff6bd044d58e854b4f9f45884e19cf7423 Mon Sep 17 00:00:00 2001 From: UlyanaAndrukhiv Date: Wed, 10 Dec 2025 16:02:40 +0200 Subject: [PATCH 13/15] Removed the validation check for required executors count --- .../access/rpc/backend/accounts/accounts.go | 16 ----- engine/access/rpc/backend/scripts/scripts.go | 6 -- engine/access/state_stream/backend/backend.go | 2 - .../backend/backend_executiondata.go | 2 - .../optimistic_sync/errors.go | 25 ------- .../execution_result/info_provider.go | 14 +--- .../execution_result/info_provider_test.go | 72 +++---------------- .../execution_result_info_provider.go | 1 - 8 files changed, 12 insertions(+), 126 deletions(-) diff --git a/engine/access/rpc/backend/accounts/accounts.go b/engine/access/rpc/backend/accounts/accounts.go index 2015896dc86..5c3caea96c3 100644 --- a/engine/access/rpc/backend/accounts/accounts.go +++ b/engine/access/rpc/backend/accounts/accounts.go @@ -127,8 +127,6 @@ func (a *Accounts) GetAccountAtLatestBlock(ctx context.Context, address flow.Add return nil, nil, access.NewDataNotFoundError("execution data", err) case common.IsInsufficientExecutionReceipts(err): return nil, nil, access.NewDataNotFoundError("execution data", err) - case optimistic_sync.IsRequiredExecutorsCountExceededError(err): - return nil, nil, access.NewInvalidRequestError(err) case optimistic_sync.IsAgreeingExecutorsCountExceededError(err): return nil, nil, access.NewInvalidRequestError(err) case optimistic_sync.IsUnknownRequiredExecutorError(err): @@ -189,8 +187,6 @@ func (a *Accounts) GetAccountAtBlockHeight( return nil, nil, access.NewDataNotFoundError("execution data", err) case common.IsInsufficientExecutionReceipts(err): return nil, nil, access.NewDataNotFoundError("execution data", err) - case optimistic_sync.IsRequiredExecutorsCountExceededError(err): - return nil, nil, access.NewInvalidRequestError(err) case optimistic_sync.IsAgreeingExecutorsCountExceededError(err): return nil, nil, access.NewInvalidRequestError(err) case optimistic_sync.IsUnknownRequiredExecutorError(err): @@ -247,8 +243,6 @@ func (a *Accounts) GetAccountBalanceAtLatestBlock(ctx context.Context, address f return 0, nil, access.NewDataNotFoundError("execution data", err) case common.IsInsufficientExecutionReceipts(err): return 0, nil, access.NewDataNotFoundError("execution data", err) - case optimistic_sync.IsRequiredExecutorsCountExceededError(err): - return 0, nil, access.NewInvalidRequestError(err) case optimistic_sync.IsAgreeingExecutorsCountExceededError(err): return 0, nil, access.NewInvalidRequestError(err) case optimistic_sync.IsUnknownRequiredExecutorError(err): @@ -309,8 +303,6 @@ func (a *Accounts) GetAccountBalanceAtBlockHeight( return 0, nil, access.NewDataNotFoundError("execution data", err) case common.IsInsufficientExecutionReceipts(err): return 0, nil, access.NewDataNotFoundError("execution data", err) - case optimistic_sync.IsRequiredExecutorsCountExceededError(err): - return 0, nil, access.NewInvalidRequestError(err) case optimistic_sync.IsAgreeingExecutorsCountExceededError(err): return 0, nil, access.NewInvalidRequestError(err) case optimistic_sync.IsUnknownRequiredExecutorError(err): @@ -371,8 +363,6 @@ func (a *Accounts) GetAccountKeyAtLatestBlock( return nil, nil, access.NewDataNotFoundError("execution data", err) case common.IsInsufficientExecutionReceipts(err): return nil, nil, access.NewDataNotFoundError("execution data", err) - case optimistic_sync.IsRequiredExecutorsCountExceededError(err): - return nil, nil, access.NewInvalidRequestError(err) case optimistic_sync.IsAgreeingExecutorsCountExceededError(err): return nil, nil, access.NewInvalidRequestError(err) case optimistic_sync.IsUnknownRequiredExecutorError(err): @@ -434,8 +424,6 @@ func (a *Accounts) GetAccountKeyAtBlockHeight( return nil, nil, access.NewDataNotFoundError("execution data", err) case common.IsInsufficientExecutionReceipts(err): return nil, nil, access.NewDataNotFoundError("execution data", err) - case optimistic_sync.IsRequiredExecutorsCountExceededError(err): - return nil, nil, access.NewInvalidRequestError(err) case optimistic_sync.IsAgreeingExecutorsCountExceededError(err): return nil, nil, access.NewInvalidRequestError(err) case optimistic_sync.IsUnknownRequiredExecutorError(err): @@ -495,8 +483,6 @@ func (a *Accounts) GetAccountKeysAtLatestBlock( return nil, nil, access.NewDataNotFoundError("execution data", err) case common.IsInsufficientExecutionReceipts(err): return nil, nil, access.NewDataNotFoundError("execution data", err) - case optimistic_sync.IsRequiredExecutorsCountExceededError(err): - return nil, nil, access.NewInvalidRequestError(err) case optimistic_sync.IsAgreeingExecutorsCountExceededError(err): return nil, nil, access.NewInvalidRequestError(err) case optimistic_sync.IsUnknownRequiredExecutorError(err): @@ -557,8 +543,6 @@ func (a *Accounts) GetAccountKeysAtBlockHeight( return nil, nil, access.NewDataNotFoundError("execution data", err) case common.IsInsufficientExecutionReceipts(err): return nil, nil, access.NewDataNotFoundError("execution data", err) - case optimistic_sync.IsRequiredExecutorsCountExceededError(err): - return nil, nil, access.NewInvalidRequestError(err) case optimistic_sync.IsAgreeingExecutorsCountExceededError(err): return nil, nil, access.NewInvalidRequestError(err) case optimistic_sync.IsUnknownRequiredExecutorError(err): diff --git a/engine/access/rpc/backend/scripts/scripts.go b/engine/access/rpc/backend/scripts/scripts.go index 17e39a65a7c..1067a8aef3e 100644 --- a/engine/access/rpc/backend/scripts/scripts.go +++ b/engine/access/rpc/backend/scripts/scripts.go @@ -163,8 +163,6 @@ func (b *Scripts) ExecuteScriptAtLatestBlock( return nil, nil, access.NewDataNotFoundError("execution data", err) case common.IsInsufficientExecutionReceipts(err): return nil, nil, access.NewDataNotFoundError("execution data", err) - case optimistic_sync.IsRequiredExecutorsCountExceededError(err): - return nil, nil, access.NewInvalidRequestError(err) case optimistic_sync.IsAgreeingExecutorsCountExceededError(err): return nil, nil, access.NewInvalidRequestError(err) case optimistic_sync.IsUnknownRequiredExecutorError(err): @@ -229,8 +227,6 @@ func (b *Scripts) ExecuteScriptAtBlockID( return nil, nil, access.NewDataNotFoundError("execution data", err) case common.IsInsufficientExecutionReceipts(err): return nil, nil, access.NewDataNotFoundError("execution data", err) - case optimistic_sync.IsRequiredExecutorsCountExceededError(err): - return nil, nil, access.NewInvalidRequestError(err) case optimistic_sync.IsAgreeingExecutorsCountExceededError(err): return nil, nil, access.NewInvalidRequestError(err) case optimistic_sync.IsUnknownRequiredExecutorError(err): @@ -300,8 +296,6 @@ func (b *Scripts) ExecuteScriptAtBlockHeight( return nil, nil, access.NewDataNotFoundError("execution data", err) case common.IsInsufficientExecutionReceipts(err): return nil, nil, access.NewDataNotFoundError("execution data", err) - case optimistic_sync.IsRequiredExecutorsCountExceededError(err): - return nil, nil, access.NewInvalidRequestError(err) case optimistic_sync.IsAgreeingExecutorsCountExceededError(err): return nil, nil, access.NewInvalidRequestError(err) case optimistic_sync.IsUnknownRequiredExecutorError(err): diff --git a/engine/access/state_stream/backend/backend.go b/engine/access/state_stream/backend/backend.go index 6574c4d8ac4..745927ab874 100644 --- a/engine/access/state_stream/backend/backend.go +++ b/engine/access/state_stream/backend/backend.go @@ -223,8 +223,6 @@ func (b *StateStreamBackend) GetRegisterValues( return nil, nil, access.NewDataNotFoundError("execution data", err) case common.IsInsufficientExecutionReceipts(err): return nil, nil, access.NewDataNotFoundError("execution data", err) - case optimistic_sync.IsRequiredExecutorsCountExceededError(err): - return nil, nil, access.NewInvalidRequestError(err) case optimistic_sync.IsAgreeingExecutorsCountExceededError(err): return nil, nil, access.NewInvalidRequestError(err) case optimistic_sync.IsUnknownRequiredExecutorError(err): diff --git a/engine/access/state_stream/backend/backend_executiondata.go b/engine/access/state_stream/backend/backend_executiondata.go index 89b13f0cddc..a4cf62644a5 100644 --- a/engine/access/state_stream/backend/backend_executiondata.go +++ b/engine/access/state_stream/backend/backend_executiondata.go @@ -61,8 +61,6 @@ func (b *ExecutionDataBackend) GetExecutionDataByBlockID( return nil, nil, access.NewDataNotFoundError("execution data", err) case common.IsInsufficientExecutionReceipts(err): return nil, nil, access.NewDataNotFoundError("execution data", err) - case optimistic_sync.IsRequiredExecutorsCountExceededError(err): - return nil, nil, access.NewInvalidRequestError(err) case optimistic_sync.IsAgreeingExecutorsCountExceededError(err): return nil, nil, access.NewInvalidRequestError(err) case optimistic_sync.IsUnknownRequiredExecutorError(err): diff --git a/module/executiondatasync/optimistic_sync/errors.go b/module/executiondatasync/optimistic_sync/errors.go index 8ddd3a05919..88deab075dc 100644 --- a/module/executiondatasync/optimistic_sync/errors.go +++ b/module/executiondatasync/optimistic_sync/errors.go @@ -7,31 +7,6 @@ import ( "github.com/onflow/flow-go/model/flow" ) -// RequiredExecutorsCountExceededError indicates that the requested number of required executors -// exceeds the total available execution nodes. -type RequiredExecutorsCountExceededError struct { - requiredExecutorsCount int - availableExecutorsCount int -} - -func NewRequiredExecutorsCountExceededError(requiredExecutorsCount int, availableExecutorsCount int) *RequiredExecutorsCountExceededError { - return &RequiredExecutorsCountExceededError{ - requiredExecutorsCount: requiredExecutorsCount, - availableExecutorsCount: availableExecutorsCount, - } -} - -func (e *RequiredExecutorsCountExceededError) Error() string { - return fmt.Sprintf("required executors count exceeded: required %d, available %d", - e.requiredExecutorsCount, e.availableExecutorsCount, - ) -} - -func IsRequiredExecutorsCountExceededError(err error) bool { - var requiredExecutorsCountExceededError *RequiredExecutorsCountExceededError - return errors.As(err, &requiredExecutorsCountExceededError) -} - // AgreeingExecutorsCountExceededError indicates that the requested number of agreeing executors // exceeds the total available execution nodes. type AgreeingExecutorsCountExceededError struct { diff --git a/module/executiondatasync/optimistic_sync/execution_result/info_provider.go b/module/executiondatasync/optimistic_sync/execution_result/info_provider.go index 558d8a56844..4867e450a04 100644 --- a/module/executiondatasync/optimistic_sync/execution_result/info_provider.go +++ b/module/executiondatasync/optimistic_sync/execution_result/info_provider.go @@ -55,7 +55,6 @@ func NewExecutionResultInfoProvider( // Expected errors during normal operations: // - [common.InsufficientExecutionReceipts]: Found insufficient receipts for given block ID. // - [storage.ErrNotFound]: If the data was not found. -// - [optimistic_sync.RequiredExecutorsCountExceededError]: Required executor IDs count exceeds available executors. // - [optimistic_sync.AgreeingExecutorsCountExceededError]: Agreeing executors count exceeds available executors. // - [optimistic_sync.UnknownRequiredExecutorError]: A required executor ID is not in the available set. // - [optimistic_sync.CriteriaNotMetError]: Returned when the block is already @@ -161,25 +160,16 @@ func (e *Provider) ExecutionResultInfo( // validateCriteria verifies that the provided optimistic sync criteria can be // satisfied by the currently available execution nodes. // -// The validation ensures that the required executor IDs do not exceed the number -// of available executors, that the requested AgreeingExecutorsCount is feasible, +// The validation ensures that the requested AgreeingExecutorsCount is feasible, // and that every required executor ID is present in the available set. // // Expected errors during normal operations: -// - [optimistic_sync.RequiredExecutorsCountExceededError]: Required executor IDs count exceeds available executors. // - [optimistic_sync.AgreeingExecutorsCountExceededError]: Agreeing executors count exceeds available executors. // - [optimistic_sync.UnknownRequiredExecutorError]: A required executor ID is not in the available set. func (e *Provider) validateCriteria( criteria optimistic_sync.Criteria, availableExecutors flow.IdentityList, ) error { - requiredExecutors := criteria.RequiredExecutors - if len(availableExecutors) < len(requiredExecutors) { - return optimistic_sync.NewRequiredExecutorsCountExceededError( - len(requiredExecutors), - len(availableExecutors), - ) - } if uint(len(availableExecutors)) < criteria.AgreeingExecutorsCount { return optimistic_sync.NewAgreeingExecutorsCountExceededError( criteria.AgreeingExecutorsCount, @@ -188,7 +178,7 @@ func (e *Provider) validateCriteria( } lookup := availableExecutors.Lookup() - for _, executorID := range requiredExecutors { + for _, executorID := range criteria.RequiredExecutors { if _, ok := lookup[executorID]; !ok { return optimistic_sync.NewUnknownRequiredExecutorError(executorID) } diff --git a/module/executiondatasync/optimistic_sync/execution_result/info_provider_test.go b/module/executiondatasync/optimistic_sync/execution_result/info_provider_test.go index 2466b4c2c1e..72872c3cf5e 100644 --- a/module/executiondatasync/optimistic_sync/execution_result/info_provider_test.go +++ b/module/executiondatasync/optimistic_sync/execution_result/info_provider_test.go @@ -11,6 +11,7 @@ import ( "github.com/onflow/flow-go/model/flow" "github.com/onflow/flow-go/module/executiondatasync/optimistic_sync" protocol "github.com/onflow/flow-go/state/protocol/mock" + "github.com/onflow/flow-go/storage" storagemock "github.com/onflow/flow-go/storage/mock" "github.com/onflow/flow-go/utils/unittest" ) @@ -106,8 +107,6 @@ func (suite *ExecutionResultInfoProviderSuite) TestExecutionResultQuery() { } suite.receipts.On("ByBlockID", block.ID()).Return(receipts, nil).Once() - suite.headers.On("ByBlockID", block.ID()).Return(block.ToHeader(), nil).Once() - suite.headers.On("BlockIDByHeight", block.Height).Return(block.ID(), nil).Once() suite.setupIdentitiesMock(allExecutionNodes) // Require specific executors (first two nodes) @@ -153,8 +152,6 @@ func (suite *ExecutionResultInfoProviderSuite) TestExecutionResultQuery() { suite.setupIdentitiesMock(allExecutionNodes) suite.receipts.On("ByBlockID", block.ID()).Return(receipts, nil).Once() - suite.headers.On("ByBlockID", block.ID()).Return(block.ToHeader(), nil).Once() - suite.headers.On("BlockIDByHeight", block.Height).Return(block.ID(), nil).Once() query, err := provider.ExecutionResultInfo(block.ID(), optimistic_sync.Criteria{}) suite.Require().NoError(err) @@ -183,6 +180,8 @@ func (suite *ExecutionResultInfoProviderSuite) TestExecutionResultQuery() { // Set up a separate mock call for this specific block suite.receipts.On("ByBlockID", insufficientBlock.ID()).Return(receipts, nil).Once() + suite.headers.On("ByBlockID", insufficientBlock.ID()).Return(insufficientBlock.ToHeader(), nil).Once() + suite.headers.On("BlockIDByHeight", insufficientBlock.Height).Return(flow.ZeroID, storage.ErrNotFound).Once() suite.setupIdentitiesMock(allExecutionNodes) result, err := provider.ExecutionResultInfo( @@ -209,6 +208,8 @@ func (suite *ExecutionResultInfoProviderSuite) TestExecutionResultQuery() { } suite.receipts.On("ByBlockID", block.ID()).Return(receipts, nil).Once() + suite.headers.On("ByBlockID", block.ID()).Return(block.ToHeader(), nil).Once() + suite.headers.On("BlockIDByHeight", block.Height).Return(flow.ZeroID, storage.ErrNotFound).Once() suite.setupIdentitiesMock(allExecutionNodes) // Require executors that didn't produce any receipts @@ -223,24 +224,6 @@ func (suite *ExecutionResultInfoProviderSuite) TestExecutionResultQuery() { }, ) - suite.Run("required executors count is greater than available executors count returns error", func() { - provider := suite.createProvider(flow.IdentifierList{}, optimistic_sync.Criteria{}) - - // setup specific executors (first two nodes) - suite.setupIdentitiesMock(allExecutionNodes[0:2]) - requiredExecutors := allExecutionNodes.NodeIDs() - - query, err := provider.ExecutionResultInfo( - block.ID(), optimistic_sync.Criteria{ - AgreeingExecutorsCount: 2, - RequiredExecutors: requiredExecutors, - }, - ) - suite.Require().Error(err) - suite.Require().Nil(query) - suite.Require().True(optimistic_sync.IsRequiredExecutorsCountExceededError(err)) - }) - suite.Run("agreeing executors count is greater than available executors count returns error", func() { provider := suite.createProvider(flow.IdentifierList{}, optimistic_sync.Criteria{}) @@ -289,48 +272,23 @@ func (suite *ExecutionResultInfoProviderSuite) TestExecutionResultQuery() { receipts[i] = r } suite.receipts.On("ByBlockID", block.ID()).Return(receipts, nil).Once() - suite.setupIdentitiesMock(allExecutionNodes) - - // Require all executors, but only one produces receipts - query, err := provider.ExecutionResultInfo( - block.ID(), optimistic_sync.Criteria{ - AgreeingExecutorsCount: 1, - RequiredExecutors: allExecutionNodes.NodeIDs(), - }, - ) - suite.Require().Error(err) - suite.Require().Nil(query) - suite.Require().True(optimistic_sync.IsCriteriaNotMetError(err)) - }) - - suite.Run("requested block conflicts with a finalized, returns error", func() { - provider := suite.createProvider(flow.IdentifierList{}, optimistic_sync.Criteria{}) - - receipts := make(flow.ExecutionReceiptList, totalReceipts) - for i := 0; i < totalReceipts; i++ { - r := unittest.ReceiptForBlockFixture(block) - r.ExecutorID = allExecutionNodes[i].NodeID - r.ExecutionResult = *executionResult - receipts[i] = r - } - suite.receipts.On("ByBlockID", block.ID()).Return(receipts, nil).Once() suite.headers.On("ByBlockID", block.ID()).Return(block.ToHeader(), nil).Once() - finalizedBlockID := unittest.IdentifierFixture() - suite.headers.On("BlockIDByHeight", block.Height).Return(finalizedBlockID, nil).Once() + suite.headers.On("BlockIDByHeight", block.Height).Return(block.ID(), nil).Once() + suite.state.On("Sealed").Return(suite.snapshot, nil).Once() + suite.snapshot.On("Head").Return(func() *flow.Header { return block.ToHeader() }, nil).Once() suite.setupIdentitiesMock(allExecutionNodes) // Require all executors, but only one produces receipts query, err := provider.ExecutionResultInfo( block.ID(), optimistic_sync.Criteria{ AgreeingExecutorsCount: 1, - RequiredExecutors: allExecutionNodes.NodeIDs(), + RequiredExecutors: allExecutionNodes[1:2].NodeIDs(), }, ) suite.Require().Error(err) suite.Require().Nil(query) - suite.Require().True(optimistic_sync.IsBlockFinalityMismatchError(err)) + suite.Require().True(optimistic_sync.IsCriteriaNotMetError(err)) }) - } // TestRootBlockHandling tests the special case handling for root blocks. @@ -394,8 +352,6 @@ func (suite *ExecutionResultInfoProviderSuite) TestPreferredAndRequiredExecution suite.Run( "with default optimistic_sync.Criteria", func() { - suite.headers.On("ByBlockID", block.ID()).Return(block.ToHeader(), nil).Once() - suite.headers.On("BlockIDByHeight", block.Height).Return(block.ID(), nil).Once() suite.setupIdentitiesMock(allExecutionNodes) provider := suite.createProvider(flow.IdentifierList{}, optimistic_sync.Criteria{}) @@ -414,8 +370,6 @@ func (suite *ExecutionResultInfoProviderSuite) TestPreferredAndRequiredExecution suite.Run( "with operator preferred executors", func() { - suite.headers.On("ByBlockID", block.ID()).Return(block.ToHeader(), nil).Once() - suite.headers.On("BlockIDByHeight", block.Height).Return(block.ID(), nil).Once() suite.setupIdentitiesMock(allExecutionNodes) provider := suite.createProvider( @@ -438,8 +392,6 @@ func (suite *ExecutionResultInfoProviderSuite) TestPreferredAndRequiredExecution suite.Run( "with operator required executors", func() { - suite.headers.On("ByBlockID", block.ID()).Return(block.ToHeader(), nil).Once() - suite.headers.On("BlockIDByHeight", block.Height).Return(block.ID(), nil).Once() suite.setupIdentitiesMock(allExecutionNodes) provider := suite.createProvider( @@ -463,8 +415,6 @@ func (suite *ExecutionResultInfoProviderSuite) TestPreferredAndRequiredExecution suite.Run( "with both: operator preferred & required executors", func() { - suite.headers.On("ByBlockID", block.ID()).Return(block.ToHeader(), nil).Once() - suite.headers.On("BlockIDByHeight", block.Height).Return(block.ID(), nil).Once() suite.setupIdentitiesMock(allExecutionNodes) provider := suite.createProvider( @@ -491,8 +441,6 @@ func (suite *ExecutionResultInfoProviderSuite) TestPreferredAndRequiredExecution suite.Run( "with client preferred executors", func() { - suite.headers.On("ByBlockID", block.ID()).Return(block.ToHeader(), nil).Once() - suite.headers.On("BlockIDByHeight", block.Height).Return(block.ID(), nil).Once() suite.setupIdentitiesMock(allExecutionNodes) provider := suite.createProvider( diff --git a/module/executiondatasync/optimistic_sync/execution_result_info_provider.go b/module/executiondatasync/optimistic_sync/execution_result_info_provider.go index 868fd5e6553..85746798d35 100644 --- a/module/executiondatasync/optimistic_sync/execution_result_info_provider.go +++ b/module/executiondatasync/optimistic_sync/execution_result_info_provider.go @@ -55,7 +55,6 @@ type ExecutionResultInfoProvider interface { // Expected error returns during normal operation: // - [common.InsufficientExecutionReceipts]: Found insufficient receipts for given block ID. // - [storage.ErrNotFound]: If the data was not found. - // - [optimistic_sync.RequiredExecutorsCountExceededError]: Required executor IDs count exceeds available executors. // - [optimistic_sync.AgreeingExecutorsCountExceededError]: Agreeing executors count exceeds available executors. // - [optimistic_sync.UnknownRequiredExecutorError]: A required executor ID is not in the available set. // - [optimistic_sync.CriteriaNotMetError]: Returned when the block is already From 8e2e7e1bffda77c379a9351d68050147a39995a0 Mon Sep 17 00:00:00 2001 From: UlyanaAndrukhiv Date: Thu, 11 Dec 2025 13:50:26 +0200 Subject: [PATCH 14/15] Moved is sealed logic for fork-aware execution result info into separate method --- .../execution_result/info_provider.go | 66 +++++++++++-------- 1 file changed, 38 insertions(+), 28 deletions(-) diff --git a/module/executiondatasync/optimistic_sync/execution_result/info_provider.go b/module/executiondatasync/optimistic_sync/execution_result/info_provider.go index 4867e450a04..e4c3ca19fdf 100644 --- a/module/executiondatasync/optimistic_sync/execution_result/info_provider.go +++ b/module/executiondatasync/optimistic_sync/execution_result/info_provider.go @@ -96,41 +96,18 @@ func (e *Provider) ExecutionResultInfo( result, executorIDs, findResultErr := e.findResultAndExecutors(blockID, criteria) if findResultErr != nil { - findResultErr = fmt.Errorf("failed to find result and executors for block ID %v: %w", blockID, findResultErr) // We want to return a more specific error when no matching execution results were found. // This helps callers understand that their criteria likely cannot be met — especially if the // block is already sealed. - - // Step 1: Get the block header - header, err := e.headers.ByBlockID(blockID) - if err != nil { - return nil, fmt.Errorf("failed to get header by block ID %v: %w", blockID, findResultErr) - } - - // Step 2a: Lookup the finalized block ID at this height - blockIDFinalized, err := e.headers.BlockIDByHeight(header.Height) + isBlockSealed, err := e.isBlockSealed(blockID) if err != nil { - return nil, fmt.Errorf("failed to lookup finalized block ID at height %d: %w", header.Height, findResultErr) + return nil, fmt.Errorf("failed to check if block sealed: %w", err) } - - // Step 2b: Check if this block is finalized. - // If BlockIDByHeight returns an ID that doesn't match, not finalized. - isFinalized := blockIDFinalized == blockID - - // Step 3: Check sealed status only if block finalized. - if isFinalized { - sealedHeader, err := e.state.Sealed().Head() - if err != nil { - return nil, fmt.Errorf("failed to lookup sealed header: %w", err) - } - - // If block is sealed, and didn't find any matching results return an error - if header.Height <= sealedHeader.Height { - return nil, optimistic_sync.NewCriteriaNotMetError(blockID) - } + if isBlockSealed { + return nil, optimistic_sync.NewCriteriaNotMetError(blockID) } - return nil, findResultErr + return nil, fmt.Errorf("failed to find result and executors for block ID %v: %w", blockID, findResultErr) } executors := executorIdentities.Filter(filter.HasNodeID[flow.Identity](executorIDs...)) @@ -157,6 +134,39 @@ func (e *Provider) ExecutionResultInfo( }, nil } +// isBlockSealed reports whether the given block is sealed. +// It returns (true, nil) if the block is sealed if, It returns (false, nil) if the block is not sealed. +// +// No errors are expected during normal operation. +func (e *Provider) isBlockSealed(blockID flow.Identifier) (bool, error) { + // Step 1: Get the block header + header, err := e.headers.ByBlockID(blockID) + if err != nil { + return false, fmt.Errorf("failed to get header for block %v: %w", blockID, err) + } + + // Step 2a: Lookup the finalized block ID at this height + blockIDFinalized, err := e.headers.BlockIDByHeight(header.Height) + if err != nil { + // no finalized block is known at given height, block is not finalized + return false, nil + } + + // Step 2b: Check if this block is finalized. + // If BlockIDByHeight returns an ID that doesn't match, block is not finalized, it cannot be sealed. + if blockIDFinalized != blockID { + return false, nil + } + + // Step 3: Check sealed status only if block finalized. + sealedHeader, err := e.state.Sealed().Head() + if err != nil { + return false, fmt.Errorf("failed to lookup sealed header: %w", err) + } + + return header.Height <= sealedHeader.Height, nil +} + // validateCriteria verifies that the provided optimistic sync criteria can be // satisfied by the currently available execution nodes. // From 80888e7ec42dfebf856122a8ec1ad0401f3f3e86 Mon Sep 17 00:00:00 2001 From: UlyanaAndrukhiv Date: Thu, 11 Dec 2025 15:09:27 +0200 Subject: [PATCH 15/15] Updated godocs --- .../optimistic_sync/execution_result/info_provider.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/module/executiondatasync/optimistic_sync/execution_result/info_provider.go b/module/executiondatasync/optimistic_sync/execution_result/info_provider.go index e4c3ca19fdf..2afa6543995 100644 --- a/module/executiondatasync/optimistic_sync/execution_result/info_provider.go +++ b/module/executiondatasync/optimistic_sync/execution_result/info_provider.go @@ -135,7 +135,7 @@ func (e *Provider) ExecutionResultInfo( } // isBlockSealed reports whether the given block is sealed. -// It returns (true, nil) if the block is sealed if, It returns (false, nil) if the block is not sealed. +// It returns (true, nil) if the block is sealed, and (false, nil) if it is not sealed. // // No errors are expected during normal operation. func (e *Provider) isBlockSealed(blockID flow.Identifier) (bool, error) {