diff --git a/cmd/access/node_builder/access_node_builder.go b/cmd/access/node_builder/access_node_builder.go index 96a6804880e..997537ed506 100644 --- a/cmd/access/node_builder/access_node_builder.go +++ b/cmd/access/node_builder/access_node_builder.go @@ -1784,6 +1784,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 d0ee103118f..23c36e8d724 100644 --- a/cmd/observer/node_builder/observer_builder.go +++ b/cmd/observer/node_builder/observer_builder.go @@ -1141,6 +1141,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 f46a49a4aeb..5c3caea96c3 100644 --- a/engine/access/rpc/backend/accounts/accounts.go +++ b/engine/access/rpc/backend/accounts/accounts.go @@ -127,6 +127,12 @@ 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.IsAgreeingExecutorsCountExceededError(err): + return nil, nil, access.NewInvalidRequestError(err) + case optimistic_sync.IsUnknownRequiredExecutorError(err): + return nil, nil, access.NewInvalidRequestError(err) + case optimistic_sync.IsCriteriaNotMetError(err): + return nil, nil, access.NewInvalidRequestError(err) default: return nil, nil, access.RequireNoError(ctx, err) } @@ -181,6 +187,12 @@ 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.IsAgreeingExecutorsCountExceededError(err): + return nil, nil, access.NewInvalidRequestError(err) + case optimistic_sync.IsUnknownRequiredExecutorError(err): + return nil, nil, access.NewInvalidRequestError(err) + case optimistic_sync.IsCriteriaNotMetError(err): + return nil, nil, access.NewInvalidRequestError(err) default: return nil, nil, access.RequireNoError(ctx, err) } @@ -231,6 +243,12 @@ 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.IsAgreeingExecutorsCountExceededError(err): + return 0, nil, access.NewInvalidRequestError(err) + case optimistic_sync.IsUnknownRequiredExecutorError(err): + return 0, nil, access.NewInvalidRequestError(err) + case optimistic_sync.IsCriteriaNotMetError(err): + return 0, nil, access.NewInvalidRequestError(err) default: return 0, nil, access.RequireNoError(ctx, err) } @@ -285,6 +303,12 @@ 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.IsAgreeingExecutorsCountExceededError(err): + return 0, nil, access.NewInvalidRequestError(err) + case optimistic_sync.IsUnknownRequiredExecutorError(err): + return 0, nil, access.NewInvalidRequestError(err) + case optimistic_sync.IsCriteriaNotMetError(err): + return 0, nil, access.NewInvalidRequestError(err) default: return 0, nil, access.RequireNoError(ctx, err) } @@ -339,6 +363,12 @@ 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.IsAgreeingExecutorsCountExceededError(err): + return nil, nil, access.NewInvalidRequestError(err) + case optimistic_sync.IsUnknownRequiredExecutorError(err): + return nil, nil, access.NewInvalidRequestError(err) + case optimistic_sync.IsCriteriaNotMetError(err): + return nil, nil, access.NewInvalidRequestError(err) default: return nil, nil, access.RequireNoError(ctx, err) } @@ -394,6 +424,12 @@ 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.IsAgreeingExecutorsCountExceededError(err): + return nil, nil, access.NewInvalidRequestError(err) + case optimistic_sync.IsUnknownRequiredExecutorError(err): + return nil, nil, access.NewInvalidRequestError(err) + case optimistic_sync.IsCriteriaNotMetError(err): + return nil, nil, access.NewInvalidRequestError(err) default: return nil, nil, access.RequireNoError(ctx, err) } @@ -447,6 +483,12 @@ 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.IsAgreeingExecutorsCountExceededError(err): + return nil, nil, access.NewInvalidRequestError(err) + case optimistic_sync.IsUnknownRequiredExecutorError(err): + return nil, nil, access.NewInvalidRequestError(err) + case optimistic_sync.IsCriteriaNotMetError(err): + return nil, nil, access.NewInvalidRequestError(err) default: return nil, nil, access.RequireNoError(ctx, err) } @@ -501,6 +543,12 @@ 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.IsAgreeingExecutorsCountExceededError(err): + return nil, nil, access.NewInvalidRequestError(err) + case optimistic_sync.IsUnknownRequiredExecutorError(err): + return nil, nil, access.NewInvalidRequestError(err) + case optimistic_sync.IsCriteriaNotMetError(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..1067a8aef3e 100644 --- a/engine/access/rpc/backend/scripts/scripts.go +++ b/engine/access/rpc/backend/scripts/scripts.go @@ -163,6 +163,12 @@ 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.IsAgreeingExecutorsCountExceededError(err): + return nil, nil, access.NewInvalidRequestError(err) + case optimistic_sync.IsUnknownRequiredExecutorError(err): + return nil, nil, access.NewInvalidRequestError(err) + case optimistic_sync.IsCriteriaNotMetError(err): + return nil, nil, access.NewInvalidRequestError(err) default: return nil, nil, access.RequireNoError(ctx, err) } @@ -221,6 +227,12 @@ 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.IsAgreeingExecutorsCountExceededError(err): + return nil, nil, access.NewInvalidRequestError(err) + case optimistic_sync.IsUnknownRequiredExecutorError(err): + return nil, nil, access.NewInvalidRequestError(err) + case optimistic_sync.IsCriteriaNotMetError(err): + return nil, nil, access.NewInvalidRequestError(err) default: return nil, nil, access.RequireNoError(ctx, err) } @@ -284,6 +296,12 @@ 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.IsAgreeingExecutorsCountExceededError(err): + return nil, nil, access.NewInvalidRequestError(err) + case optimistic_sync.IsUnknownRequiredExecutorError(err): + return nil, nil, access.NewInvalidRequestError(err) + case optimistic_sync.IsCriteriaNotMetError(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..745927ab874 100644 --- a/engine/access/state_stream/backend/backend.go +++ b/engine/access/state_stream/backend/backend.go @@ -223,6 +223,12 @@ 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.IsAgreeingExecutorsCountExceededError(err): + return nil, nil, access.NewInvalidRequestError(err) + case optimistic_sync.IsUnknownRequiredExecutorError(err): + return nil, nil, access.NewInvalidRequestError(err) + case optimistic_sync.IsCriteriaNotMetError(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..a4cf62644a5 100644 --- a/engine/access/state_stream/backend/backend_executiondata.go +++ b/engine/access/state_stream/backend/backend_executiondata.go @@ -61,6 +61,12 @@ 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.IsAgreeingExecutorsCountExceededError(err): + return nil, nil, access.NewInvalidRequestError(err) + case optimistic_sync.IsUnknownRequiredExecutorError(err): + return nil, nil, access.NewInvalidRequestError(err) + case optimistic_sync.IsCriteriaNotMetError(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 new file mode 100644 index 00000000000..88deab075dc --- /dev/null +++ b/module/executiondatasync/optimistic_sync/errors.go @@ -0,0 +1,73 @@ +package optimistic_sync + +import ( + "errors" + "fmt" + + "github.com/onflow/flow-go/model/flow" +) + +// AgreeingExecutorsCountExceededError indicates that the requested number of agreeing executors +// exceeds the total available execution nodes. +type AgreeingExecutorsCountExceededError struct { + agreeingExecutorsCount uint + availableExecutorsCount int +} + +func NewAgreeingExecutorsCountExceededError(agreeingExecutorsCount uint, availableExecutorsCount int) *AgreeingExecutorsCountExceededError { + return &AgreeingExecutorsCountExceededError{ + agreeingExecutorsCount: agreeingExecutorsCount, + availableExecutorsCount: availableExecutorsCount, + } +} + +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 { + 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 { + 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 and no execution result satisfies the criteria", 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 9657d1cc565..2afa6543995 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,17 +54,25 @@ 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. +// - [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 +// sealed but no execution result can satisfy the provided criteria. 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) } + err = e.validateCriteria(criteria, 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 { @@ -83,9 +94,20 @@ 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 { + // 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. + isBlockSealed, err := e.isBlockSealed(blockID) + if err != nil { + return nil, fmt.Errorf("failed to check if block sealed: %w", err) + } + if isBlockSealed { + return nil, optimistic_sync.NewCriteriaNotMetError(blockID) + } + + 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...)) @@ -112,6 +134,69 @@ func (e *Provider) ExecutionResultInfo( }, nil } +// isBlockSealed reports whether the given block is 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) { + // 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. +// +// 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.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 uint(len(availableExecutors)) < criteria.AgreeingExecutorsCount { + return optimistic_sync.NewAgreeingExecutorsCountExceededError( + criteria.AgreeingExecutorsCount, + len(availableExecutors), + ) + } + + lookup := availableExecutors.Lookup() + for _, executorID := range criteria.RequiredExecutors { + if _, ok := lookup[executorID]; !ok { + return optimistic_sync.NewUnknownRequiredExecutorError(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. 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..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" ) @@ -25,6 +26,7 @@ type ExecutionResultInfoProviderSuite struct { log zerolog.Logger receipts *storagemock.ExecutionReceipts + headers *storagemock.Headers rootBlock *flow.Block rootBlockResult *flow.ExecutionResult @@ -42,7 +44,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)) @@ -51,7 +53,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() } @@ -63,6 +64,7 @@ func (suite *ExecutionResultInfoProviderSuite) createProvider( suite.log, suite.state, suite.receipts, + suite.headers, NewExecutionNodeSelector(preferredExecutors, operatorCriteria.RequiredExecutors), operatorCriteria, ) @@ -75,7 +77,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 +106,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,8 +150,8 @@ func (suite *ExecutionResultInfoProviderSuite) TestExecutionResultQuery() { receipts[i] = r } - suite.receipts.On("ByBlockID", block.ID()).Return(receipts, nil) suite.setupIdentitiesMock(allExecutionNodes) + suite.receipts.On("ByBlockID", block.ID()).Return(receipts, nil).Once() query, err := provider.ExecutionResultInfo(block.ID(), optimistic_sync.Criteria{}) suite.Require().NoError(err) @@ -178,16 +180,18 @@ 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) - _, 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,37 +202,102 @@ 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 } - suite.receipts.On("ByBlockID", block.ID()).Return(receipts, nil) + 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 - _, 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("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{}) + + 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(optimistic_sync.IsUnknownRequiredExecutorError(err)) + }) + + suite.Run("criteria not met 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.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[1:2].NodeIDs(), + }, + ) + suite.Require().Error(err) + suite.Require().Nil(query) + suite.Require().True(optimistic_sync.IsCriteriaNotMetError(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 +314,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 +349,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 +370,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 +392,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 +415,8 @@ 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 +441,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(), diff --git a/module/executiondatasync/optimistic_sync/execution_result_info_provider.go b/module/executiondatasync/optimistic_sync/execution_result_info_provider.go index 4a47b31a0d6..85746798d35 100644 --- a/module/executiondatasync/optimistic_sync/execution_result_info_provider.go +++ b/module/executiondatasync/optimistic_sync/execution_result_info_provider.go @@ -54,7 +54,10 @@ 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. + // - [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 + // sealed but no execution result can satisfy the provided criteria. ExecutionResultInfo(blockID flow.Identifier, criteria Criteria) (*ExecutionResultInfo, error) }