diff --git a/admin/README.md b/admin/README.md index 9da63d0831e..4603b90c7e8 100644 --- a/admin/README.md +++ b/admin/README.md @@ -115,3 +115,8 @@ curl localhost:9002/admin/run_command -H 'Content-Type: application/json' -d '{" curl localhost:9002/admin/run_command -H 'Content-Type: application/json' -d '{"commandName": "protocol-snapshot"}' curl localhost:9002/admin/run_command -H 'Content-Type: application/json' -d '{"commandName": "protocol-snapshot", "data": { "blocks-to-skip": 10 }}' ``` + +### To backfill transaction error messages +``` +curl localhost:9002/admin/run_command -H 'Content-Type: application/json' -d '{"commandName": "backfill-tx-error-messages", "data": { "start-height": 340, "end-height": 343, "execution-node-ids":["ec7b934df29248d574ae1cc33ae77f22f0fcf96a79e009224c46374d1837824e", "8cbdc8d24a28899a33140cb68d4146cd6f2f6c18c57f54c299f26351d126919e"] }}' +``` diff --git a/admin/commands/storage/backfill_tx_error_messages.go b/admin/commands/storage/backfill_tx_error_messages.go new file mode 100644 index 00000000000..6f059e88469 --- /dev/null +++ b/admin/commands/storage/backfill_tx_error_messages.go @@ -0,0 +1,239 @@ +package storage + +import ( + "context" + "fmt" + + execproto "github.com/onflow/flow/protobuf/go/flow/execution" + + "github.com/onflow/flow-go/admin" + "github.com/onflow/flow-go/admin/commands" + "github.com/onflow/flow-go/engine/access/index" + "github.com/onflow/flow-go/engine/access/rpc/backend" + commonrpc "github.com/onflow/flow-go/engine/common/rpc" + "github.com/onflow/flow-go/engine/common/rpc/convert" + "github.com/onflow/flow-go/model/flow" + "github.com/onflow/flow-go/model/flow/filter" + "github.com/onflow/flow-go/state/protocol" + "github.com/onflow/flow-go/storage" +) + +var _ commands.AdminCommand = (*BackfillTxErrorMessagesCommand)(nil) + +// backfillTxErrorMessagesRequest represents the input parameters for +// backfilling transaction error messages. +type backfillTxErrorMessagesRequest struct { + startHeight uint64 // Start height from which to begin backfilling. + endHeight uint64 // End height up to which backfilling is performed. + executionNodeIds flow.IdentityList // List of execution node IDs to be used for backfilling. +} + +// BackfillTxErrorMessagesCommand executes a command to backfill +// transaction error messages by fetching them from execution nodes. +type BackfillTxErrorMessagesCommand struct { + state protocol.State + txResultsIndex *index.TransactionResultsIndex + txErrorMessages storage.TransactionResultErrorMessages + backend *backend.Backend +} + +// NewBackfillTxErrorMessagesCommand creates a new instance of BackfillTxErrorMessagesCommand +func NewBackfillTxErrorMessagesCommand( + state protocol.State, + txResultsIndex *index.TransactionResultsIndex, + txErrorMessages storage.TransactionResultErrorMessages, + backend *backend.Backend, +) commands.AdminCommand { + return &BackfillTxErrorMessagesCommand{ + state: state, + txResultsIndex: txResultsIndex, + txErrorMessages: txErrorMessages, + backend: backend, + } +} + +// Validator validates the input for the backfill command. The input is validated +// for field types, boundaries, and coherence of start and end heights. +// +// Expected errors during normal operation: +// - admin.InvalidAdminReqError - if start-height is greater than end-height or +// if the input format is invalid, if an invalid execution node ID is provided. +func (b *BackfillTxErrorMessagesCommand) Validator(request *admin.CommandRequest) error { + input, ok := request.Data.(map[string]interface{}) + if !ok { + return admin.NewInvalidAdminReqFormatError("expected map[string]any") + } + + data := &backfillTxErrorMessagesRequest{} + + rootHeight := b.state.Params().SealedRoot().Height + data.startHeight = rootHeight // Default value + + if startHeightIn, ok := input["start-height"]; ok { + if startHeight, err := parseN(startHeightIn); err != nil { + return admin.NewInvalidAdminReqErrorf("invalid 'start-height' field: %w", err) + } else if startHeight > rootHeight { + data.startHeight = startHeight + } + } + + sealed, err := b.state.Sealed().Head() + if err != nil { + return fmt.Errorf("failed to lookup sealed header: %w", err) + } + data.endHeight = sealed.Height // Default value + + if endHeightIn, ok := input["end-height"]; ok { + if endHeight, err := parseN(endHeightIn); err != nil { + return admin.NewInvalidAdminReqErrorf("invalid 'end-height' field: %w", err) + } else if endHeight < sealed.Height { + data.endHeight = endHeight + } + } + + if data.endHeight < data.startHeight { + return admin.NewInvalidAdminReqErrorf("start-height %d should not be smaller than end-height %d", data.startHeight, data.endHeight) + } + + identities, err := b.state.Final().Identities(filter.HasRole[flow.Identity](flow.RoleExecution)) + if err != nil { + return fmt.Errorf("failed to retreive execution IDs: %w", err) + } + + if executionNodeIdsIn, ok := input["execution-node-ids"]; ok { + executionNodeIds, err := b.parseExecutionNodeIds(executionNodeIdsIn, identities) + if err != nil { + return err + } + data.executionNodeIds = executionNodeIds + } else { + // in case no execution node ids provided, the command will use any valid execution node + data.executionNodeIds = identities + } + + request.ValidatorData = data + + return nil +} + +// Handler performs the backfilling operation by fetching missing transaction +// error messages for blocks within the specified height range. Uses execution nodes +// from data.executionNodeIds if available, otherwise defaults to valid execution nodes. +// +// No errors are expected during normal operation. +func (b *BackfillTxErrorMessagesCommand) Handler(ctx context.Context, request *admin.CommandRequest) (interface{}, error) { + if b.txErrorMessages == nil { + return nil, fmt.Errorf("failed to backfill, could not get transaction error messages storage") + } + + data := request.ValidatorData.(*backfillTxErrorMessagesRequest) + + for height := data.startHeight; height <= data.endHeight; height++ { + header, err := b.state.AtHeight(height).Head() + if err != nil { + return nil, fmt.Errorf("failed to get block header: %w", err) + } + + blockID := header.ID() + + exists, err := b.txErrorMessages.Exists(blockID) + if err != nil { + return nil, fmt.Errorf("could not check existance of transaction result error messages: %w", err) + } + + if exists { + continue + } + + results, err := b.txResultsIndex.ByBlockID(blockID, height) + if err != nil { + return nil, fmt.Errorf("failed to get result by block ID: %w", err) + } + + fetchTxErrorMessages := false + for _, txResult := range results { + if txResult.Failed { + fetchTxErrorMessages = true + } + } + + if !fetchTxErrorMessages { + continue + } + + req := &execproto.GetTransactionErrorMessagesByBlockIDRequest{ + BlockId: convert.IdentifierToMessage(blockID), + } + + resp, execNode, err := b.backend.GetTransactionErrorMessagesFromAnyEN(ctx, data.executionNodeIds.ToSkeleton(), req) + if err != nil { + return nil, fmt.Errorf("failed to retrieve transaction error messages for block id %#v: %w", blockID, err) + } + + err = b.storeTransactionResultErrorMessages(blockID, resp, execNode) + if err != nil { + return nil, fmt.Errorf("could not store error messages: %w", err) + } + } + + return nil, nil +} + +// parseExecutionNodeIds converts a list of node IDs from input to flow.IdentityList. +// Returns an error if the IDs are invalid or empty. +// +// Expected errors during normal operation: +// - admin.InvalidAdminReqParameterError - if execution-node-ids is empty or has an invalid format. +func (b *BackfillTxErrorMessagesCommand) parseExecutionNodeIds(executionNodeIdsIn interface{}, allIdentities flow.IdentityList) (flow.IdentityList, error) { + var ids flow.IdentityList + + switch executionNodeIds := executionNodeIdsIn.(type) { + case []string: + if len(executionNodeIds) == 0 { + return nil, admin.NewInvalidAdminReqParameterError("execution-node-ids", "must be a non empty list of string", executionNodeIdsIn) + } + requestedENIdentifiers, err := commonrpc.IdentifierList(executionNodeIds) + if err != nil { + return nil, admin.NewInvalidAdminReqParameterError("execution-node-ids", err.Error(), executionNodeIdsIn) + } + + for _, en := range requestedENIdentifiers { + id, exists := allIdentities.ByNodeID(en) + if !exists { + return nil, admin.NewInvalidAdminReqParameterError("execution-node-ids", "could not found execution nodes by provided ids", executionNodeIdsIn) + } + ids = append(ids, id) + } + default: + return nil, admin.NewInvalidAdminReqParameterError("execution-node-ids", "must be a list of string", executionNodeIdsIn) + } + + return ids, nil +} + +// storeTransactionResultErrorMessages saves retrieved error messages for a given block ID. +// +// No errors are expected during normal operation. +func (b *BackfillTxErrorMessagesCommand) storeTransactionResultErrorMessages( + blockID flow.Identifier, + errorMessagesResponses []*execproto.GetTransactionErrorMessagesResponse_Result, + execNode *flow.IdentitySkeleton, +) error { + errorMessages := make([]flow.TransactionResultErrorMessage, 0, len(errorMessagesResponses)) + for _, value := range errorMessagesResponses { + errorMessage := flow.TransactionResultErrorMessage{ + ErrorMessage: value.ErrorMessage, + TransactionID: convert.MessageToIdentifier(value.TransactionId), + Index: value.Index, + ExecutorID: execNode.NodeID, + } + errorMessages = append(errorMessages, errorMessage) + } + + err := b.txErrorMessages.Store(blockID, errorMessages) + if err != nil { + return fmt.Errorf("failed to store transaction error messages: %w", err) + } + + return nil +} diff --git a/admin/commands/storage/backfill_tx_error_messages_test.go b/admin/commands/storage/backfill_tx_error_messages_test.go new file mode 100644 index 00000000000..425fa766671 --- /dev/null +++ b/admin/commands/storage/backfill_tx_error_messages_test.go @@ -0,0 +1,519 @@ +package storage + +import ( + "context" + "fmt" + "os" + "testing" + + execproto "github.com/onflow/flow/protobuf/go/flow/execution" + "github.com/rs/zerolog" + "github.com/stretchr/testify/mock" + "github.com/stretchr/testify/require" + "github.com/stretchr/testify/suite" + + "github.com/onflow/flow-go/admin" + "github.com/onflow/flow-go/admin/commands" + "github.com/onflow/flow-go/engine/access/index" + accessmock "github.com/onflow/flow-go/engine/access/mock" + "github.com/onflow/flow-go/engine/access/rpc/backend" + connectionmock "github.com/onflow/flow-go/engine/access/rpc/connection/mock" + commonrpc "github.com/onflow/flow-go/engine/common/rpc" + "github.com/onflow/flow-go/model/flow" + syncmock "github.com/onflow/flow-go/module/state_synchronization/mock" + "github.com/onflow/flow-go/state/protocol" + "github.com/onflow/flow-go/state/protocol/invalid" + protocolmock "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" + unittestMocks "github.com/onflow/flow-go/utils/unittest/mocks" +) + +const expectedErrorMsg = "expected test error" + +type BackfillTxErrorMessagesSuite struct { + suite.Suite + + command commands.AdminCommand + + log zerolog.Logger + state *protocolmock.State + snapshot *protocolmock.Snapshot + params *protocolmock.Params + + txErrorMessages *storagemock.TransactionResultErrorMessages + transactionResults *storagemock.LightTransactionResults + receipts *storagemock.ExecutionReceipts + headers *storagemock.Headers + + execClient *accessmock.ExecutionAPIClient + + connFactory *connectionmock.ConnectionFactory + reporter *syncmock.IndexReporter + allENIDs flow.IdentityList + + backend *backend.Backend + + blockHeadersMap map[uint64]*flow.Header + + nodeRootBlock flow.Block + blockCount int +} + +func TestBackfillTxErrorMessages(t *testing.T) { + t.Parallel() + suite.Run(t, new(BackfillTxErrorMessagesSuite)) +} + +func (suite *BackfillTxErrorMessagesSuite) SetupTest() { + suite.log = zerolog.New(os.Stderr) + + suite.state = new(protocolmock.State) + suite.headers = new(storagemock.Headers) + suite.receipts = new(storagemock.ExecutionReceipts) + suite.transactionResults = storagemock.NewLightTransactionResults(suite.T()) + suite.txErrorMessages = new(storagemock.TransactionResultErrorMessages) + suite.execClient = new(accessmock.ExecutionAPIClient) + + suite.blockCount = 5 + suite.blockHeadersMap = make(map[uint64]*flow.Header, suite.blockCount) + suite.nodeRootBlock = unittest.BlockFixture() + suite.nodeRootBlock.Header.Height = 0 + suite.blockHeadersMap[suite.nodeRootBlock.Header.Height] = suite.nodeRootBlock.Header + + parent := suite.nodeRootBlock.Header + + for i := 1; i <= suite.blockCount; i++ { + block := unittest.BlockWithParentFixture(parent) + // update for next iteration + parent = block.Header + suite.blockHeadersMap[block.Header.Height] = block.Header + } + + suite.params = protocolmock.NewParams(suite.T()) + suite.params.On("SealedRoot").Return(suite.nodeRootBlock.Header, nil) + suite.state.On("Params").Return(suite.params, nil).Maybe() + + suite.snapshot = createSnapshot(parent) + suite.state.On("Sealed").Return(suite.snapshot) + suite.state.On("Final").Return(suite.snapshot) + + suite.state.On("AtHeight", mock.Anything).Return( + func(height uint64) protocol.Snapshot { + if int(height) < len(suite.blockHeadersMap) { + header := suite.blockHeadersMap[height] + return createSnapshot(header) + } + return invalid.NewSnapshot(fmt.Errorf("invalid height: %v", height)) + }, + ) + + // Mock the protocol snapshot to return fixed execution node IDs. + suite.allENIDs = unittest.IdentityListFixture(1, unittest.WithRole(flow.RoleExecution)) + suite.snapshot.On("Identities", mock.Anything).Return( + func(flow.IdentityFilter[flow.Identity]) (flow.IdentityList, error) { + return suite.allENIDs, nil + }, nil) + + // create a mock connection factory + suite.connFactory = connectionmock.NewConnectionFactory(suite.T()) + + // Create a mock index reporter + suite.reporter = syncmock.NewIndexReporter(suite.T()) + + txResultsIndex := index.NewTransactionResultsIndex(index.NewReporter(), suite.transactionResults) + err := txResultsIndex.Initialize(suite.reporter) + suite.Require().NoError(err) + + suite.backend, err = backend.New(backend.Params{ + State: suite.state, + ExecutionReceipts: suite.receipts, + ConnFactory: suite.connFactory, + MaxHeightRange: backend.DefaultMaxHeightRange, + Log: suite.log, + SnapshotHistoryLimit: backend.DefaultSnapshotHistoryLimit, + Communicator: backend.NewNodeCommunicator(false), + ScriptExecutionMode: backend.IndexQueryModeExecutionNodesOnly, + TxResultQueryMode: backend.IndexQueryModeExecutionNodesOnly, + ChainID: flow.Testnet, + ExecNodeIdentitiesProvider: commonrpc.NewExecutionNodeIdentitiesProvider( + suite.log, + suite.state, + suite.receipts, + nil, + nil, + ), + }) + require.NoError(suite.T(), err) + + suite.command = NewBackfillTxErrorMessagesCommand( + suite.state, + txResultsIndex, + suite.txErrorMessages, + suite.backend, + ) +} + +// TestValidateInvalidFormat validates that invalid input formats trigger appropriate error responses. +// It tests several invalid cases such as: +// - Invalid "start-height" and "end-height" fields where values are in an incorrect format or out of valid ranges. +// - Invalid combinations of "start-height" and "end-height" where logical constraints are violated. +// - Invalid types for "execution-node-ids" which must be a list of strings, and invalid node IDs. +func (suite *BackfillTxErrorMessagesSuite) TestValidateInvalidFormat() { + // invalid start-height + suite.Run("invalid start-height field", func() { + err := suite.command.Validator(&admin.CommandRequest{ + Data: map[string]interface{}{ + "start-height": "123", + }, + }) + suite.Error(err) + suite.Equal(err, admin.NewInvalidAdminReqErrorf( + "invalid 'start-height' field: %w", + fmt.Errorf("invalid value for \"n\": %v", 0))) + }) + + // invalid end-height + suite.Run("invalid end-height field", func() { + err := suite.command.Validator(&admin.CommandRequest{ + Data: map[string]interface{}{ + "end-height": "123", + }, + }) + suite.Error(err) + suite.Equal(err, admin.NewInvalidAdminReqErrorf( + "invalid 'end-height' field: %w", + fmt.Errorf("invalid value for \"n\": %v", 0))) + }) + + suite.Run("invalid combination of start-height and end-height fields", func() { + startHeight := 10 + endHeight := 1 + err := suite.command.Validator(&admin.CommandRequest{ + Data: map[string]interface{}{ + "start-height": float64(startHeight), // raw json parses to float64 + "end-height": float64(endHeight), // raw json parses to float64 + }, + }) + suite.Error(err) + suite.Equal(err, admin.NewInvalidAdminReqErrorf( + "start-height %v should not be smaller than end-height %v", startHeight, endHeight)) + }) + + // invalid execution-node-ids param + suite.Run("invalid execution-node-ids field", func() { + // invalid type + err := suite.command.Validator(&admin.CommandRequest{ + Data: map[string]interface{}{ + "execution-node-ids": []int{1, 2, 3}, + }, + }) + suite.Error(err) + suite.Equal(err, admin.NewInvalidAdminReqParameterError( + "execution-node-ids", "must be a list of string", []int{1, 2, 3})) + + // invalid type + err = suite.command.Validator(&admin.CommandRequest{ + Data: map[string]interface{}{ + "execution-node-ids": "123", + }, + }) + suite.Error(err) + suite.Equal(err, admin.NewInvalidAdminReqParameterError( + "execution-node-ids", "must be a list of string", "123")) + + // invalid execution node id + invalidENID := unittest.IdentifierFixture() + err = suite.command.Validator(&admin.CommandRequest{ + Data: map[string]interface{}{ + "start-height": float64(1), // raw json parses to float64 + "end-height": float64(10), // raw json parses to float64 + "execution-node-ids": []string{invalidENID.String()}, + }, + }) + suite.Error(err) + suite.Equal(err, admin.NewInvalidAdminReqParameterError( + "execution-node-ids", "could not found execution nodes by provided ids", []string{invalidENID.String()})) + }) +} + +// TestValidateValidFormat verifies that valid input parameters result in no validation errors +// in the command validator. +// It tests various valid cases, such as: +// - Default parameters (start-height, end-height, execution-node-ids) are used. +// - Provided "start-height" and "end-height" values are within expected ranges. +// - Proper "execution-node-ids" are supplied. +func (suite *BackfillTxErrorMessagesSuite) TestValidateValidFormat() { + // start-height and end-height are not provided, the root block and the latest sealed block + // will be used as the start and end heights respectively. + // execution-node-ids is not provided, any valid execution node will be used. + suite.Run("happy case, all default parameters", func() { + err := suite.command.Validator(&admin.CommandRequest{ + Data: map[string]interface{}{}, + }) + suite.NoError(err) + }) + + // all parameters are provided + // start-height is less than root block, the root block + // will be used as the start-height. + suite.Run("happy case, start-height is less than root block", func() { + suite.params.On("SealedRoot").Return(suite.blockHeadersMap[1].Height, nil) + suite.state.On("Params").Return(suite.params, nil).Maybe() + err := suite.command.Validator(&admin.CommandRequest{ + Data: map[string]interface{}{ + "start-height": float64(2), // raw json parses to float64 + "end-height": float64(5), // raw json parses to float64 + "execution-node-ids": []string{suite.allENIDs[0].ID().String()}, + }, + }) + suite.NoError(err) + }) + + // all parameters are provided + // end-height is bigger than latest sealed block, the latest sealed block + // will be used as the end-height. + suite.Run("happy case, end-height is bigger than latest sealed block", func() { + err := suite.command.Validator(&admin.CommandRequest{ + Data: map[string]interface{}{ + "start-height": float64(1), // raw json parses to float64 + "end-height": float64(100), // raw json parses to float64 + "execution-node-ids": []string{suite.allENIDs[0].ID().String()}, + }, + }) + suite.NoError(err) + }) + + // all parameters are provided + suite.Run("happy case, all parameters are provided", func() { + err := suite.command.Validator(&admin.CommandRequest{ + Data: map[string]interface{}{ + "start-height": float64(1), // raw json parses to float64 + "end-height": float64(3), // raw json parses to float64 + "execution-node-ids": []string{suite.allENIDs[0].ID().String()}, + }, + }) + suite.NoError(err) + }) +} + +// TestHandleBackfillTxErrorMessages handles the transaction error backfill logic for different scenarios. +// It validates behavior when transaction error messages exist or do not exist in the database, handling both default and custom parameters. +func (suite *BackfillTxErrorMessagesSuite) TestHandleBackfillTxErrorMessages() { + ctx, cancel := context.WithCancel(context.Background()) + defer cancel() + + // default parameters + req := &admin.CommandRequest{ + Data: map[string]interface{}{}, + } + suite.Require().NoError(suite.command.Validator(req)) + + suite.Run("happy case, all default parameters, tx error messages do not exist in db", func() { + suite.reporter.On("LowestIndexedHeight").Return(suite.nodeRootBlock.Header.Height, nil) + suite.reporter.On("HighestIndexedHeight").Return(suite.blockHeadersMap[uint64(suite.blockCount)].Height, nil) + + // Create a mock execution client to simulate communication with execution nodes. + suite.connFactory.On("GetExecutionAPIClient", mock.Anything).Return(suite.execClient, &unittestMocks.MockCloser{}, nil) + + for i := suite.nodeRootBlock.Header.Height; i <= suite.blockHeadersMap[uint64(suite.blockCount)].Height; i++ { + blockId := suite.blockHeadersMap[i].ID() + + // Setup mock storing the transaction error message after retrieving the failed result. + suite.txErrorMessages.On("Exists", blockId).Return(false, nil).Once() + + results := suite.generateResultsForBlock() + + // Setup mock that the transaction result exists and is failed. + suite.transactionResults.On("ByBlockID", blockId). + Return(results, nil).Once() + + // Mock the execution node API calls to fetch the error messages. + suite.mockTransactionErrorMessagesResponseByBlockID(blockId, results) + + // Setup mock storing the transaction error message after retrieving the failed result. + suite.mockStoreTxErrorMessages(blockId, results, suite.allENIDs[0].ID()) + } + + _, err := suite.command.Handler(ctx, req) + suite.Require().NoError(err) + suite.assertAllExpectations() + }) + + suite.Run("happy case, all default parameters, tx error messages exist in db", func() { + for i := suite.nodeRootBlock.Header.Height; i <= suite.blockHeadersMap[uint64(suite.blockCount)].Height; i++ { + blockId := suite.blockHeadersMap[i].ID() + + // Setup mock storing the transaction error message after retrieving the failed result. + suite.txErrorMessages.On("Exists", blockId).Return(true, nil).Once() + } + + _, err := suite.command.Handler(ctx, req) + suite.Require().NoError(err) + suite.assertAllExpectations() + }) + + suite.Run("happy case, all custom parameters, tx error messages do not exist in db", func() { + // custom parameters + startHeight := 1 + endHeight := 4 + + suite.allENIDs = unittest.IdentityListFixture(3, unittest.WithRole(flow.RoleExecution)) + + executorID := suite.allENIDs[1].ID() + req = &admin.CommandRequest{ + Data: map[string]interface{}{ + "start-height": float64(startHeight), // raw json parses to float64 + "end-height": float64(endHeight), // raw json parses to float64 + "execution-node-ids": []string{executorID.String()}, + }, + } + suite.Require().NoError(suite.command.Validator(req)) + + suite.reporter.On("LowestIndexedHeight").Return(suite.nodeRootBlock.Header.Height, nil) + suite.reporter.On("HighestIndexedHeight").Return(suite.blockHeadersMap[uint64(endHeight)].Height, nil) + + // Create a mock execution client to simulate communication with execution nodes. + suite.connFactory.On("GetExecutionAPIClient", mock.Anything).Return(suite.execClient, &unittestMocks.MockCloser{}, nil) + + for i := startHeight; i <= endHeight; i++ { + blockId := suite.blockHeadersMap[uint64(i)].ID() + + // Setup mock storing the transaction error message after retrieving the failed result. + suite.txErrorMessages.On("Exists", blockId).Return(false, nil).Once() + + results := suite.generateResultsForBlock() + + // Setup mock that the transaction result exists and is failed. + suite.transactionResults.On("ByBlockID", blockId). + Return(results, nil).Once() + + // Mock the execution node API calls to fetch the error messages. + suite.mockTransactionErrorMessagesResponseByBlockID(blockId, results) + + // Setup mock storing the transaction error message after retrieving the failed result. + suite.mockStoreTxErrorMessages(blockId, results, executorID) + } + + _, err := suite.command.Handler(ctx, req) + suite.Require().NoError(err) + suite.assertAllExpectations() + }) +} + +// TestHandleBackfillTxErrorMessagesErrors tests error scenarios. +// It validates error handling in cases where transaction results are not indexed, ensuring that +// appropriate errors are returned when blocks are not indexed. +func (suite *BackfillTxErrorMessagesSuite) TestHandleBackfillTxErrorMessagesErrors() { + ctx, cancel := context.WithCancel(context.Background()) + defer cancel() + + // default parameters + req := &admin.CommandRequest{ + Data: map[string]interface{}{}, + } + suite.Require().NoError(suite.command.Validator(req)) + + suite.Run("failed, not indexed tx results", func() { + suite.reporter.On("LowestIndexedHeight").Return(suite.nodeRootBlock.Header.Height, nil) + suite.reporter.On("HighestIndexedHeight").Return(suite.nodeRootBlock.Header.Height, nil) + + expectedErr := fmt.Errorf("%w: block not indexed yet", storage.ErrHeightNotIndexed) + + blockID := suite.nodeRootBlock.Header.ID() + suite.txErrorMessages.On("Exists", blockID).Return(false, nil).Once() + + // Setup mock that the transaction result exists and is failed. + suite.transactionResults.On("ByBlockID", blockID). + Return(nil, expectedErr).Once() + + _, err := suite.command.Handler(ctx, req) + suite.Require().Error(err) + suite.Equal(err, fmt.Errorf("failed to get result by block ID: %w", expectedErr)) + suite.assertAllExpectations() + }) +} + +// generateResultsForBlock generates mock transaction results for a block. +// It creates a mix of failed and non-failed transaction results to simulate different transaction outcomes. +func (suite *BackfillTxErrorMessagesSuite) generateResultsForBlock() []flow.LightTransactionResult { + results := make([]flow.LightTransactionResult, 0) + + for i := 0; i < 5; i++ { + results = append(results, flow.LightTransactionResult{ + TransactionID: unittest.IdentifierFixture(), + Failed: i%2 == 0, // create a mix of failed and non-failed transactions + ComputationUsed: 0, + }) + } + + return results +} + +// mockTransactionErrorMessagesResponseByBlockID mocks the response of transaction error messages +// by block ID for failed transactions. It simulates API calls that retrieve error messages from execution nodes. +func (suite *BackfillTxErrorMessagesSuite) mockTransactionErrorMessagesResponseByBlockID( + blockID flow.Identifier, + results []flow.LightTransactionResult, +) { + exeEventReq := &execproto.GetTransactionErrorMessagesByBlockIDRequest{ + BlockId: blockID[:], + } + + exeErrMessagesResp := &execproto.GetTransactionErrorMessagesResponse{} + for i, result := range results { + r := result + if r.Failed { + errMsg := fmt.Sprintf("%s.%s", expectedErrorMsg, r.TransactionID) + exeErrMessagesResp.Results = append(exeErrMessagesResp.Results, &execproto.GetTransactionErrorMessagesResponse_Result{ + TransactionId: r.TransactionID[:], + ErrorMessage: errMsg, + Index: uint32(i), + }) + } + } + + suite.execClient.On("GetTransactionErrorMessagesByBlockID", mock.Anything, exeEventReq). + Return(exeErrMessagesResp, nil). + Once() +} + +// mockStoreTxErrorMessages mocks the process of storing transaction error messages in the database +// after retrieving the results of failed transactions . +func (suite *BackfillTxErrorMessagesSuite) mockStoreTxErrorMessages( + blockID flow.Identifier, + results []flow.LightTransactionResult, + executorID flow.Identifier, +) { + var txErrorMessages []flow.TransactionResultErrorMessage + + for i, result := range results { + r := result + if r.Failed { + errMsg := fmt.Sprintf("%s.%s", expectedErrorMsg, r.TransactionID) + + txErrorMessages = append(txErrorMessages, + flow.TransactionResultErrorMessage{ + TransactionID: result.TransactionID, + ErrorMessage: errMsg, + Index: uint32(i), + ExecutorID: executorID, + }) + } + } + + suite.txErrorMessages.On("Store", blockID, txErrorMessages).Return(nil).Once() +} + +// assertAllExpectations asserts that all the expectations set on various mocks are met, +// ensuring the test results are valid. +func (suite *BackfillTxErrorMessagesSuite) assertAllExpectations() { + suite.snapshot.AssertExpectations(suite.T()) + suite.state.AssertExpectations(suite.T()) + suite.headers.AssertExpectations(suite.T()) + suite.execClient.AssertExpectations(suite.T()) + suite.transactionResults.AssertExpectations(suite.T()) + suite.reporter.AssertExpectations(suite.T()) + suite.txErrorMessages.AssertExpectations(suite.T()) +} diff --git a/admin/commands/storage/read_blocks_test.go b/admin/commands/storage/read_blocks_test.go index 334260fe1a8..2b052f356e5 100644 --- a/admin/commands/storage/read_blocks_test.go +++ b/admin/commands/storage/read_blocks_test.go @@ -38,8 +38,8 @@ func TestReadBlocks(t *testing.T) { suite.Run(t, new(ReadBlocksSuite)) } -func createSnapshot(head *flow.Header) protocol.Snapshot { - snapshot := &protocolmock.Snapshot{} +func createSnapshot(head *flow.Header) *protocolmock.Snapshot { + snapshot := new(protocolmock.Snapshot) snapshot.On("Head").Return( func() *flow.Header { return head diff --git a/cmd/access/node_builder/access_node_builder.go b/cmd/access/node_builder/access_node_builder.go index 1da04c85888..91769a12c5b 100644 --- a/cmd/access/node_builder/access_node_builder.go +++ b/cmd/access/node_builder/access_node_builder.go @@ -56,6 +56,7 @@ import ( "github.com/onflow/flow-go/engine/access/subscription" followereng "github.com/onflow/flow-go/engine/common/follower" "github.com/onflow/flow-go/engine/common/requester" + commonrpc "github.com/onflow/flow-go/engine/common/rpc" "github.com/onflow/flow-go/engine/common/stop" synceng "github.com/onflow/flow-go/engine/common/synchronization" "github.com/onflow/flow-go/engine/common/version" @@ -163,7 +164,6 @@ type AccessNodeConfig struct { executionDataConfig edrequester.ExecutionDataConfig PublicNetworkConfig PublicNetworkConfig TxResultCacheSize uint - TxErrorMessagesCacheSize uint executionDataIndexingEnabled bool registersDBPath string checkpointFile string @@ -175,6 +175,7 @@ type AccessNodeConfig struct { programCacheSize uint checkPayerBalanceMode string versionControlEnabled bool + storeTxResultErrorMessages bool stopControlEnabled bool registerDBPruneThreshold uint64 } @@ -248,7 +249,6 @@ func DefaultAccessNodeConfig() *AccessNodeConfig { apiRatelimits: nil, apiBurstlimits: nil, TxResultCacheSize: 0, - TxErrorMessagesCacheSize: 1000, PublicNetworkConfig: PublicNetworkConfig{ BindAddress: cmd.NotSet, Metrics: metrics.NewNoopCollector(), @@ -280,6 +280,7 @@ func DefaultAccessNodeConfig() *AccessNodeConfig { programCacheSize: 0, checkPayerBalanceMode: accessNode.Disabled.String(), versionControlEnabled: true, + storeTxResultErrorMessages: false, stopControlEnabled: false, registerDBPruneThreshold: pruner.DefaultThreshold, } @@ -351,6 +352,9 @@ type FlowAccessNodeBuilder struct { stateStreamGrpcServer *grpcserver.GrpcServer stateStreamBackend *statestreambackend.StateStreamBackend + nodeBackend *backend.Backend + + ExecNodeIdentitiesProvider *commonrpc.ExecutionNodeIdentitiesProvider } func (builder *FlowAccessNodeBuilder) buildFollowerState() *FlowAccessNodeBuilder { @@ -1241,7 +1245,6 @@ func (builder *FlowAccessNodeBuilder) extraFlags() { flags.BoolVar(&builder.retryEnabled, "retry-enabled", defaultConfig.retryEnabled, "whether to enable the retry mechanism at the access node level") flags.BoolVar(&builder.rpcMetricsEnabled, "rpc-metrics-enabled", defaultConfig.rpcMetricsEnabled, "whether to enable the rpc metrics") flags.UintVar(&builder.TxResultCacheSize, "transaction-result-cache-size", defaultConfig.TxResultCacheSize, "transaction result cache size.(Disabled by default i.e 0)") - flags.UintVar(&builder.TxErrorMessagesCacheSize, "transaction-error-messages-cache-size", defaultConfig.TxErrorMessagesCacheSize, "transaction error messages cache size.(By default 1000)") flags.StringVarP(&builder.nodeInfoFile, "node-info-file", "", @@ -1375,6 +1378,7 @@ func (builder *FlowAccessNodeBuilder) extraFlags() { "tx-result-query-mode", defaultConfig.rpcConf.BackendConfig.TxResultQueryMode, "mode to use when querying transaction results. one of [local-only, execution-nodes-only(default), failover]") + flags.BoolVar(&builder.storeTxResultErrorMessages, "store-tx-result-error-messages", defaultConfig.storeTxResultErrorMessages, "whether enable storing the transaction error messages into the db") // Script Execution flags.StringVar(&builder.rpcConf.BackendConfig.ScriptExecutionMode, @@ -1488,9 +1492,6 @@ func (builder *FlowAccessNodeBuilder) extraFlags() { return errors.New("circuit-breaker-restore-timeout must be greater than 0") } } - if builder.TxErrorMessagesCacheSize == 0 { - return errors.New("transaction-error-messages-cache-size must be greater than 0") - } if builder.checkPayerBalanceMode != accessNode.Disabled.String() && !builder.executionDataIndexingEnabled { return errors.New("execution-data-indexing-enabled must be set if check-payer-balance is enabled") @@ -1787,6 +1788,13 @@ func (builder *FlowAccessNodeBuilder) Build() (cmd.Node, error) { builder.Storage.Events = bstorage.NewEvents(node.Metrics.Cache, node.DB) return nil }). + Module("transaction result error messages storage", func(node *cmd.NodeConfig) error { + if builder.storeTxResultErrorMessages { + builder.Storage.TransactionResultErrorMessages = bstorage.NewTransactionResultErrorMessages(node.Metrics.Cache, node.DB, bstorage.DefaultCacheSize) + } + + return nil + }). Module("reporter", func(node *cmd.NodeConfig) error { builder.Reporter = index.NewReporter() return nil @@ -1817,6 +1825,13 @@ func (builder *FlowAccessNodeBuilder) Build() (cmd.Node, error) { return nil }). + Module("transaction result error messages storage", func(node *cmd.NodeConfig) error { + if builder.storeTxResultErrorMessages { + builder.Storage.TransactionResultErrorMessages = bstorage.NewTransactionResultErrorMessages(node.Metrics.Cache, node.DB, bstorage.DefaultCacheSize) + } + + return nil + }). Component("version control", func(node *cmd.NodeConfig) (module.ReadyDoneAware, error) { if !builder.versionControlEnabled { noop := &module.NoopReadyDoneAware{} @@ -1944,33 +1959,49 @@ func (builder *FlowAccessNodeBuilder) Build() (cmd.Node, error) { } - nodeBackend, err := backend.New(backend.Params{ - State: node.State, - CollectionRPC: builder.CollectionRPC, - HistoricalAccessNodes: builder.HistoricalAccessRPCs, - Blocks: node.Storage.Blocks, - Headers: node.Storage.Headers, - Collections: node.Storage.Collections, - Transactions: node.Storage.Transactions, - ExecutionReceipts: node.Storage.Receipts, - ExecutionResults: node.Storage.Results, - ChainID: node.RootChainID, - AccessMetrics: builder.AccessMetrics, - ConnFactory: connFactory, - RetryEnabled: builder.retryEnabled, - MaxHeightRange: backendConfig.MaxHeightRange, - PreferredExecutionNodeIDs: backendConfig.PreferredExecutionNodeIDs, - FixedExecutionNodeIDs: backendConfig.FixedExecutionNodeIDs, - Log: node.Logger, - SnapshotHistoryLimit: backend.DefaultSnapshotHistoryLimit, - Communicator: backend.NewNodeCommunicator(backendConfig.CircuitBreakerConfig.Enabled), - TxResultCacheSize: builder.TxResultCacheSize, - TxErrorMessagesCacheSize: builder.TxErrorMessagesCacheSize, - ScriptExecutor: builder.ScriptExecutor, - ScriptExecutionMode: scriptExecMode, - CheckPayerBalanceMode: checkPayerBalanceMode, - EventQueryMode: eventQueryMode, - BlockTracker: blockTracker, + preferredENIdentifiers, err := commonrpc.IdentifierList(backendConfig.PreferredExecutionNodeIDs) + if err != nil { + return nil, fmt.Errorf("failed to convert node id string to Flow Identifier for preferred EN map: %w", err) + } + + fixedENIdentifiers, err := commonrpc.IdentifierList(backendConfig.FixedExecutionNodeIDs) + if err != nil { + return nil, fmt.Errorf("failed to convert node id string to Flow Identifier for fixed EN map: %w", err) + } + + builder.ExecNodeIdentitiesProvider = commonrpc.NewExecutionNodeIdentitiesProvider( + node.Logger, + node.State, + node.Storage.Receipts, + preferredENIdentifiers, + fixedENIdentifiers, + ) + + builder.nodeBackend, err = backend.New(backend.Params{ + State: node.State, + CollectionRPC: builder.CollectionRPC, + HistoricalAccessNodes: builder.HistoricalAccessRPCs, + Blocks: node.Storage.Blocks, + Headers: node.Storage.Headers, + Collections: node.Storage.Collections, + Transactions: node.Storage.Transactions, + ExecutionReceipts: node.Storage.Receipts, + ExecutionResults: node.Storage.Results, + TxResultErrorMessages: node.Storage.TransactionResultErrorMessages, + ChainID: node.RootChainID, + AccessMetrics: builder.AccessMetrics, + ConnFactory: connFactory, + RetryEnabled: builder.retryEnabled, + MaxHeightRange: backendConfig.MaxHeightRange, + Log: node.Logger, + SnapshotHistoryLimit: backend.DefaultSnapshotHistoryLimit, + Communicator: backend.NewNodeCommunicator(backendConfig.CircuitBreakerConfig.Enabled), + TxResultCacheSize: builder.TxResultCacheSize, + ScriptExecutor: builder.ScriptExecutor, + ScriptExecutionMode: scriptExecMode, + CheckPayerBalanceMode: checkPayerBalanceMode, + EventQueryMode: eventQueryMode, + BlockTracker: blockTracker, SubscriptionHandler: subscription.NewSubscriptionHandler( builder.Logger, broadcaster, @@ -1978,12 +2009,13 @@ func (builder *FlowAccessNodeBuilder) Build() (cmd.Node, error) { builder.stateStreamConf.ResponseLimit, builder.stateStreamConf.ClientSendBufferSize, ), - EventsIndex: builder.EventsIndex, - TxResultQueryMode: txResultQueryMode, - TxResultsIndex: builder.TxResultsIndex, - LastFullBlockHeight: lastFullBlockHeight, - IndexReporter: indexReporter, - VersionControl: builder.VersionControl, + EventsIndex: builder.EventsIndex, + TxResultQueryMode: txResultQueryMode, + TxResultsIndex: builder.TxResultsIndex, + LastFullBlockHeight: lastFullBlockHeight, + IndexReporter: indexReporter, + VersionControl: builder.VersionControl, + ExecNodeIdentitiesProvider: builder.ExecNodeIdentitiesProvider, }) if err != nil { return nil, fmt.Errorf("could not initialize backend: %w", err) @@ -1997,8 +2029,8 @@ func (builder *FlowAccessNodeBuilder) Build() (cmd.Node, error) { builder.AccessMetrics, builder.rpcMetricsEnabled, builder.Me, - nodeBackend, - nodeBackend, + builder.nodeBackend, + builder.nodeBackend, builder.secureGrpcServer, builder.unsecureGrpcServer, builder.stateStreamBackend, @@ -2020,6 +2052,14 @@ func (builder *FlowAccessNodeBuilder) Build() (cmd.Node, error) { return builder.RpcEng, nil }). + AdminCommand("backfill-tx-error-messages", func(config *cmd.NodeConfig) commands.AdminCommand { + return storageCommands.NewBackfillTxErrorMessagesCommand( + builder.State, + builder.TxResultsIndex, + builder.Storage.TransactionResultErrorMessages, + builder.nodeBackend, + ) + }). Component("ingestion engine", func(node *cmd.NodeConfig) (module.ReadyDoneAware, error) { var err error @@ -2049,9 +2089,12 @@ func (builder *FlowAccessNodeBuilder) Build() (cmd.Node, error) { node.Storage.Transactions, node.Storage.Results, node.Storage.Receipts, + node.Storage.TransactionResultErrorMessages, builder.collectionExecutedMetric, processedBlockHeight, lastFullBlockHeight, + builder.nodeBackend, + builder.ExecNodeIdentitiesProvider, ) if err != nil { return nil, err diff --git a/cmd/observer/node_builder/observer_builder.go b/cmd/observer/node_builder/observer_builder.go index 21d8211daa6..f35da6858f1 100644 --- a/cmd/observer/node_builder/observer_builder.go +++ b/cmd/observer/node_builder/observer_builder.go @@ -54,6 +54,7 @@ import ( statestreambackend "github.com/onflow/flow-go/engine/access/state_stream/backend" "github.com/onflow/flow-go/engine/access/subscription" "github.com/onflow/flow-go/engine/common/follower" + commonrpc "github.com/onflow/flow-go/engine/common/rpc" "github.com/onflow/flow-go/engine/common/stop" synceng "github.com/onflow/flow-go/engine/common/synchronization" "github.com/onflow/flow-go/engine/common/version" @@ -1951,25 +1952,41 @@ func (builder *ObserverServiceBuilder) enqueueRPCServer() { indexReporter = builder.Reporter } + preferredENIdentifiers, err := commonrpc.IdentifierList(backendConfig.PreferredExecutionNodeIDs) + if err != nil { + return nil, fmt.Errorf("failed to convert node id string to Flow Identifier for preferred EN map: %w", err) + } + + fixedENIdentifiers, err := commonrpc.IdentifierList(backendConfig.FixedExecutionNodeIDs) + if err != nil { + return nil, fmt.Errorf("failed to convert node id string to Flow Identifier for fixed EN map: %w", err) + } + + execNodeIdentitiesProvider := commonrpc.NewExecutionNodeIdentitiesProvider( + node.Logger, + node.State, + node.Storage.Receipts, + preferredENIdentifiers, + fixedENIdentifiers, + ) + backendParams := backend.Params{ - State: node.State, - Blocks: node.Storage.Blocks, - Headers: node.Storage.Headers, - Collections: node.Storage.Collections, - Transactions: node.Storage.Transactions, - ExecutionReceipts: node.Storage.Receipts, - ExecutionResults: node.Storage.Results, - ChainID: node.RootChainID, - AccessMetrics: accessMetrics, - ConnFactory: connFactory, - RetryEnabled: false, - MaxHeightRange: backendConfig.MaxHeightRange, - PreferredExecutionNodeIDs: backendConfig.PreferredExecutionNodeIDs, - FixedExecutionNodeIDs: backendConfig.FixedExecutionNodeIDs, - Log: node.Logger, - SnapshotHistoryLimit: backend.DefaultSnapshotHistoryLimit, - Communicator: backend.NewNodeCommunicator(backendConfig.CircuitBreakerConfig.Enabled), - BlockTracker: blockTracker, + State: node.State, + Blocks: node.Storage.Blocks, + Headers: node.Storage.Headers, + Collections: node.Storage.Collections, + Transactions: node.Storage.Transactions, + ExecutionReceipts: node.Storage.Receipts, + ExecutionResults: node.Storage.Results, + ChainID: node.RootChainID, + AccessMetrics: accessMetrics, + ConnFactory: connFactory, + RetryEnabled: false, + MaxHeightRange: backendConfig.MaxHeightRange, + Log: node.Logger, + SnapshotHistoryLimit: backend.DefaultSnapshotHistoryLimit, + Communicator: backend.NewNodeCommunicator(backendConfig.CircuitBreakerConfig.Enabled), + BlockTracker: blockTracker, SubscriptionHandler: subscription.NewSubscriptionHandler( builder.Logger, broadcaster, @@ -1977,8 +1994,9 @@ func (builder *ObserverServiceBuilder) enqueueRPCServer() { builder.stateStreamConf.ResponseLimit, builder.stateStreamConf.ClientSendBufferSize, ), - IndexReporter: indexReporter, - VersionControl: builder.VersionControl, + IndexReporter: indexReporter, + VersionControl: builder.VersionControl, + ExecNodeIdentitiesProvider: execNodeIdentitiesProvider, } if builder.localServiceAPIEnabled { diff --git a/engine/access/access_test.go b/engine/access/access_test.go index d7ea12e613c..16508b81cb2 100644 --- a/engine/access/access_test.go +++ b/engine/access/access_test.go @@ -26,6 +26,7 @@ import ( "github.com/onflow/flow-go/engine/access/rpc/backend" connectionmock "github.com/onflow/flow-go/engine/access/rpc/connection/mock" "github.com/onflow/flow-go/engine/access/subscription" + commonrpc "github.com/onflow/flow-go/engine/common/rpc" "github.com/onflow/flow-go/engine/common/rpc/convert" "github.com/onflow/flow-go/model/flow" "github.com/onflow/flow-go/model/flow/factory" @@ -267,10 +268,6 @@ func (suite *Suite) TestSendExpiredTransaction() { }) } -type mockCloser struct{} - -func (mc *mockCloser) Close() error { return nil } - // TestSendTransactionToRandomCollectionNode tests that collection nodes are chosen from the appropriate cluster when // forwarding transactions by sending two transactions bound for two different collection clusters. func (suite *Suite) TestSendTransactionToRandomCollectionNode() { @@ -325,20 +322,19 @@ func (suite *Suite) TestSendTransactionToRandomCollectionNode() { // create a mock connection factory connFactory := connectionmock.NewConnectionFactory(suite.T()) - connFactory.On("GetAccessAPIClient", collNode1.Address, nil).Return(col1ApiClient, &mockCloser{}, nil) - connFactory.On("GetAccessAPIClient", collNode2.Address, nil).Return(col2ApiClient, &mockCloser{}, nil) + connFactory.On("GetAccessAPIClient", collNode1.Address, nil).Return(col1ApiClient, &mocks.MockCloser{}, nil) + connFactory.On("GetAccessAPIClient", collNode2.Address, nil).Return(col2ApiClient, &mocks.MockCloser{}, nil) bnd, err := backend.New(backend.Params{State: suite.state, - Collections: collections, - Transactions: transactions, - ChainID: suite.chainID, - AccessMetrics: metrics, - ConnFactory: connFactory, - MaxHeightRange: backend.DefaultMaxHeightRange, - Log: suite.log, - SnapshotHistoryLimit: backend.DefaultSnapshotHistoryLimit, - Communicator: backend.NewNodeCommunicator(false), - TxErrorMessagesCacheSize: 1000, + Collections: collections, + Transactions: transactions, + ChainID: suite.chainID, + AccessMetrics: metrics, + ConnFactory: connFactory, + MaxHeightRange: backend.DefaultMaxHeightRange, + Log: suite.log, + SnapshotHistoryLimit: backend.DefaultSnapshotHistoryLimit, + Communicator: backend.NewNodeCommunicator(false), }) require.NoError(suite.T(), err) @@ -630,7 +626,7 @@ func (suite *Suite) TestGetSealedTransaction() { // create a mock connection factory connFactory := connectionmock.NewConnectionFactory(suite.T()) - connFactory.On("GetExecutionAPIClient", mock.Anything).Return(suite.execClient, &mockCloser{}, nil) + connFactory.On("GetExecutionAPIClient", mock.Anything).Return(suite.execClient, &mocks.MockCloser{}, nil) // initialize storage metrics := metrics.NewNoopCollector() @@ -643,24 +639,32 @@ func (suite *Suite) TestGetSealedTransaction() { blocksToMarkExecuted, err := stdmap.NewTimes(100) require.NoError(suite.T(), err) - bnd, err := backend.New(backend.Params{State: suite.state, - CollectionRPC: suite.collClient, - Blocks: all.Blocks, - Headers: all.Headers, - Collections: collections, - Transactions: transactions, - ExecutionReceipts: receipts, - ExecutionResults: results, - ChainID: suite.chainID, - AccessMetrics: suite.metrics, - ConnFactory: connFactory, - MaxHeightRange: backend.DefaultMaxHeightRange, - PreferredExecutionNodeIDs: enNodeIDs.Strings(), - Log: suite.log, - SnapshotHistoryLimit: backend.DefaultSnapshotHistoryLimit, - Communicator: backend.NewNodeCommunicator(false), - TxErrorMessagesCacheSize: 1000, - TxResultQueryMode: backend.IndexQueryModeExecutionNodesOnly, + execNodeIdentitiesProvider := commonrpc.NewExecutionNodeIdentitiesProvider( + suite.log, + suite.state, + receipts, + enNodeIDs, + nil, + ) + + bnd, err := backend.New(backend.Params{ + State: suite.state, + CollectionRPC: suite.collClient, + Blocks: all.Blocks, + Headers: all.Headers, + Collections: collections, + Transactions: transactions, + ExecutionReceipts: receipts, + ExecutionResults: results, + ChainID: suite.chainID, + AccessMetrics: suite.metrics, + ConnFactory: connFactory, + MaxHeightRange: backend.DefaultMaxHeightRange, + Log: suite.log, + SnapshotHistoryLimit: backend.DefaultSnapshotHistoryLimit, + Communicator: backend.NewNodeCommunicator(false), + TxResultQueryMode: backend.IndexQueryModeExecutionNodesOnly, + ExecNodeIdentitiesProvider: execNodeIdentitiesProvider, }) require.NoError(suite.T(), err) @@ -686,7 +690,25 @@ func (suite *Suite) TestGetSealedTransaction() { // create the ingest engine processedHeight := bstorage.NewConsumerProgress(db, module.ConsumeProgressIngestionEngineBlockHeight) - ingestEng, err := ingestion.New(suite.log, suite.net, suite.state, suite.me, suite.request, all.Blocks, all.Headers, collections, transactions, results, receipts, collectionExecutedMetric, processedHeight, lastFullBlockHeight) + ingestEng, err := ingestion.New( + suite.log, + suite.net, + suite.state, + suite.me, + suite.request, + all.Blocks, + all.Headers, + collections, + transactions, + results, + receipts, + nil, + collectionExecutedMetric, + processedHeight, + lastFullBlockHeight, + bnd, + execNodeIdentitiesProvider, + ) require.NoError(suite.T(), err) // 1. Assume that follower engine updated the block storage and the protocol state. The block is reported as sealed @@ -743,7 +765,6 @@ func (suite *Suite) TestGetTransactionResult() { all := util.StorageLayer(suite.T(), db) results := bstorage.NewExecutionResults(suite.metrics, db) receipts := bstorage.NewExecutionReceipts(suite.metrics, db, results, bstorage.DefaultCacheSize) - originID := unittest.IdentifierFixture() *suite.state = protocol.State{} @@ -790,7 +811,7 @@ func (suite *Suite) TestGetTransactionResult() { // create a mock connection factory connFactory := connectionmock.NewConnectionFactory(suite.T()) - connFactory.On("GetExecutionAPIClient", mock.Anything).Return(suite.execClient, &mockCloser{}, nil) + connFactory.On("GetExecutionAPIClient", mock.Anything).Return(suite.execClient, &mocks.MockCloser{}, nil) // initialize storage metrics := metrics.NewNoopCollector() @@ -805,24 +826,31 @@ func (suite *Suite) TestGetTransactionResult() { blocksToMarkExecuted, err := stdmap.NewTimes(100) require.NoError(suite.T(), err) + execNodeIdentitiesProvider := commonrpc.NewExecutionNodeIdentitiesProvider( + suite.log, + suite.state, + receipts, + enNodeIDs, + nil, + ) + bnd, err := backend.New(backend.Params{State: suite.state, - CollectionRPC: suite.collClient, - Blocks: all.Blocks, - Headers: all.Headers, - Collections: collections, - Transactions: transactions, - ExecutionReceipts: receipts, - ExecutionResults: results, - ChainID: suite.chainID, - AccessMetrics: suite.metrics, - ConnFactory: connFactory, - MaxHeightRange: backend.DefaultMaxHeightRange, - PreferredExecutionNodeIDs: enNodeIDs.Strings(), - Log: suite.log, - SnapshotHistoryLimit: backend.DefaultSnapshotHistoryLimit, - Communicator: backend.NewNodeCommunicator(false), - TxErrorMessagesCacheSize: 1000, - TxResultQueryMode: backend.IndexQueryModeExecutionNodesOnly, + CollectionRPC: suite.collClient, + Blocks: all.Blocks, + Headers: all.Headers, + Collections: collections, + Transactions: transactions, + ExecutionReceipts: receipts, + ExecutionResults: results, + ChainID: suite.chainID, + AccessMetrics: suite.metrics, + ConnFactory: connFactory, + MaxHeightRange: backend.DefaultMaxHeightRange, + Log: suite.log, + SnapshotHistoryLimit: backend.DefaultSnapshotHistoryLimit, + Communicator: backend.NewNodeCommunicator(false), + TxResultQueryMode: backend.IndexQueryModeExecutionNodesOnly, + ExecNodeIdentitiesProvider: execNodeIdentitiesProvider, }) require.NoError(suite.T(), err) @@ -847,7 +875,25 @@ func (suite *Suite) TestGetTransactionResult() { require.NoError(suite.T(), err) // create the ingest engine - ingestEng, err := ingestion.New(suite.log, suite.net, suite.state, suite.me, suite.request, all.Blocks, all.Headers, collections, transactions, results, receipts, collectionExecutedMetric, processedHeight, lastFullBlockHeight) + ingestEng, err := ingestion.New( + suite.log, + suite.net, + suite.state, + suite.me, + suite.request, + all.Blocks, + all.Headers, + collections, + transactions, + results, + receipts, + nil, + collectionExecutedMetric, + processedHeight, + lastFullBlockHeight, + bnd, + execNodeIdentitiesProvider, + ) require.NoError(suite.T(), err) background, cancel := context.WithCancel(context.Background()) @@ -1009,36 +1055,42 @@ func (suite *Suite) TestExecuteScript() { collections := bstorage.NewCollections(db, transactions) results := bstorage.NewExecutionResults(suite.metrics, db) receipts := bstorage.NewExecutionReceipts(suite.metrics, db, results, bstorage.DefaultCacheSize) - identities := unittest.IdentityListFixture(2, unittest.WithRole(flow.RoleExecution)) suite.sealedSnapshot.On("Identities", mock.Anything).Return(identities, nil) suite.finalSnapshot.On("Identities", mock.Anything).Return(identities, nil) // create a mock connection factory connFactory := connectionmock.NewConnectionFactory(suite.T()) - connFactory.On("GetExecutionAPIClient", mock.Anything).Return(suite.execClient, &mockCloser{}, nil) + connFactory.On("GetExecutionAPIClient", mock.Anything).Return(suite.execClient, &mocks.MockCloser{}, nil) + + execNodeIdentitiesProvider := commonrpc.NewExecutionNodeIdentitiesProvider( + suite.log, + suite.state, + receipts, + nil, + identities.NodeIDs(), + ) var err error suite.backend, err = backend.New(backend.Params{ - State: suite.state, - CollectionRPC: suite.collClient, - Blocks: all.Blocks, - Headers: all.Headers, - Collections: collections, - Transactions: transactions, - ExecutionReceipts: receipts, - ExecutionResults: results, - ChainID: suite.chainID, - AccessMetrics: suite.metrics, - ConnFactory: connFactory, - MaxHeightRange: backend.DefaultMaxHeightRange, - FixedExecutionNodeIDs: (identities.NodeIDs()).Strings(), - Log: suite.log, - SnapshotHistoryLimit: backend.DefaultSnapshotHistoryLimit, - Communicator: backend.NewNodeCommunicator(false), - ScriptExecutionMode: backend.IndexQueryModeExecutionNodesOnly, - TxErrorMessagesCacheSize: 1000, - TxResultQueryMode: backend.IndexQueryModeExecutionNodesOnly, + State: suite.state, + CollectionRPC: suite.collClient, + Blocks: all.Blocks, + Headers: all.Headers, + Collections: collections, + Transactions: transactions, + ExecutionReceipts: receipts, + ExecutionResults: results, + ChainID: suite.chainID, + AccessMetrics: suite.metrics, + ConnFactory: connFactory, + MaxHeightRange: backend.DefaultMaxHeightRange, + Log: suite.log, + SnapshotHistoryLimit: backend.DefaultSnapshotHistoryLimit, + Communicator: backend.NewNodeCommunicator(false), + ScriptExecutionMode: backend.IndexQueryModeExecutionNodesOnly, + TxResultQueryMode: backend.IndexQueryModeExecutionNodesOnly, + ExecNodeIdentitiesProvider: execNodeIdentitiesProvider, }) require.NoError(suite.T(), err) @@ -1076,7 +1128,25 @@ func (suite *Suite) TestExecuteScript() { require.NoError(suite.T(), err) // create the ingest engine - ingestEng, err := ingestion.New(suite.log, suite.net, suite.state, suite.me, suite.request, all.Blocks, all.Headers, collections, transactions, results, receipts, collectionExecutedMetric, processedHeight, lastFullBlockHeight) + ingestEng, err := ingestion.New( + suite.log, + suite.net, + suite.state, + suite.me, + suite.request, + all.Blocks, + all.Headers, + collections, + transactions, + results, + receipts, + nil, + collectionExecutedMetric, + processedHeight, + lastFullBlockHeight, + suite.backend, + execNodeIdentitiesProvider, + ) require.NoError(suite.T(), err) // create another block as a predecessor of the block created earlier diff --git a/engine/access/ingestion/engine.go b/engine/access/ingestion/engine.go index b36e5598c59..f7736b104e5 100644 --- a/engine/access/ingestion/engine.go +++ b/engine/access/ingestion/engine.go @@ -6,11 +6,15 @@ import ( "fmt" "time" + execproto "github.com/onflow/flow/protobuf/go/flow/execution" "github.com/rs/zerolog" "github.com/onflow/flow-go/consensus/hotstuff/model" "github.com/onflow/flow-go/engine" + "github.com/onflow/flow-go/engine/access/rpc/backend" "github.com/onflow/flow-go/engine/common/fifoqueue" + commonrpc "github.com/onflow/flow-go/engine/common/rpc" + "github.com/onflow/flow-go/engine/common/rpc/convert" "github.com/onflow/flow-go/model/flow" "github.com/onflow/flow-go/model/flow/filter" "github.com/onflow/flow-go/module" @@ -81,6 +85,9 @@ type Engine struct { // Notifier for queue consumer finalizedBlockNotifier engine.Notifier + // txResultErrorMessagesChan is used to fetch and store transaction result error messages for blocks + txResultErrorMessagesChan chan flow.Identifier + log zerolog.Logger // used to log relevant actions with context state protocol.State // used to access the protocol state me module.Local // used to access local node information @@ -88,17 +95,21 @@ type Engine struct { // storage // FIX: remove direct DB access by substituting indexer module - blocks storage.Blocks - headers storage.Headers - collections storage.Collections - transactions storage.Transactions - executionReceipts storage.ExecutionReceipts - maxReceiptHeight uint64 - executionResults storage.ExecutionResults + blocks storage.Blocks + headers storage.Headers + collections storage.Collections + transactions storage.Transactions + executionReceipts storage.ExecutionReceipts + maxReceiptHeight uint64 + executionResults storage.ExecutionResults + transactionResultErrorMessages storage.TransactionResultErrorMessages lastFullBlockHeight *counters.PersistentStrictMonotonicCounter // metrics collectionExecutedMetric module.CollectionExecutedMetric + + execNodeIdentitiesProvider *commonrpc.ExecutionNodeIdentitiesProvider + backend *backend.Backend } var _ network.MessageProcessor = (*Engine)(nil) @@ -118,9 +129,12 @@ func New( transactions storage.Transactions, executionResults storage.ExecutionResults, executionReceipts storage.ExecutionReceipts, + transactionResultErrorMessages storage.TransactionResultErrorMessages, collectionExecutedMetric module.CollectionExecutedMetric, processedHeight storage.ConsumerProgress, lastFullBlockHeight *counters.PersistentStrictMonotonicCounter, + backend *backend.Backend, + execNodeIdentitiesProvider *commonrpc.ExecutionNodeIdentitiesProvider, ) (*Engine, error) { executionReceiptsRawQueue, err := fifoqueue.NewFifoQueue(defaultQueueCapacity) if err != nil { @@ -145,25 +159,29 @@ func New( // initialize the propagation engine with its dependencies e := &Engine{ - log: log.With().Str("engine", "ingestion").Logger(), - state: state, - me: me, - request: request, - blocks: blocks, - headers: headers, - collections: collections, - transactions: transactions, - executionResults: executionResults, - executionReceipts: executionReceipts, - maxReceiptHeight: 0, - collectionExecutedMetric: collectionExecutedMetric, - finalizedBlockNotifier: engine.NewNotifier(), - lastFullBlockHeight: lastFullBlockHeight, + log: log.With().Str("engine", "ingestion").Logger(), + state: state, + me: me, + request: request, + blocks: blocks, + headers: headers, + collections: collections, + transactions: transactions, + executionResults: executionResults, + executionReceipts: executionReceipts, + transactionResultErrorMessages: transactionResultErrorMessages, + maxReceiptHeight: 0, + collectionExecutedMetric: collectionExecutedMetric, + finalizedBlockNotifier: engine.NewNotifier(), + lastFullBlockHeight: lastFullBlockHeight, // queue / notifier for execution receipts - executionReceiptsNotifier: engine.NewNotifier(), - executionReceiptsQueue: executionReceiptsQueue, - messageHandler: messageHandler, + executionReceiptsNotifier: engine.NewNotifier(), + txResultErrorMessagesChan: make(chan flow.Identifier, 1), + executionReceiptsQueue: executionReceiptsQueue, + messageHandler: messageHandler, + backend: backend, + execNodeIdentitiesProvider: execNodeIdentitiesProvider, } // jobqueue Jobs object that tracks finalized blocks by height. This is used by the finalizedBlockConsumer @@ -196,6 +214,7 @@ func New( AddWorker(e.processBackground). AddWorker(e.processExecutionReceipts). AddWorker(e.runFinalizedBlockConsumer). + AddWorker(e.processTransactionResultErrorMessages). Build() // register engine with the execution receipt provider @@ -241,7 +260,7 @@ func (e *Engine) processFinalizedBlockJob(ctx irrecoverable.SignalerContext, job ctx.Throw(fmt.Errorf("failed to convert job to block: %w", err)) } - err = e.processFinalizedBlock(block) + err = e.processFinalizedBlock(ctx, block) if err == nil { done() return @@ -337,9 +356,124 @@ func (e *Engine) processAvailableExecutionReceipts(ctx context.Context) error { if err := e.handleExecutionReceipt(msg.OriginID, receipt); err != nil { return err } + + // notify, to fetch and store transaction result error messages for block + e.txResultErrorMessagesChan <- receipt.BlockID + } +} + +// processTransactionResultErrorMessages handles error messages related to transaction +// results by reading from the error messages channel and processing them accordingly. +// +// This function listens for messages on the txResultErrorMessagesChan channel and +// processes each transaction result error message as it arrives. +// +// No errors are expected during normal operation. +func (e *Engine) processTransactionResultErrorMessages(ctx irrecoverable.SignalerContext, ready component.ReadyFunc) { + ready() + + for { + select { + case <-ctx.Done(): + return + case blockID := <-e.txResultErrorMessagesChan: + err := e.handleTransactionResultErrorMessages(ctx, blockID) + if err != nil { + // if an error reaches this point, it is unexpected + ctx.Throw(err) + return + } + } } } +// handleTransactionResultErrorMessages processes transaction result error messages for a given block ID. +// It retrieves error messages from the backend if they do not already exist in storage. +// +// The function first checks if error messages for the given block ID are already present in storage. +// If they are not, it fetches the messages from execution nodes and stores them. +// +// Parameters: +// - ctx: The context for managing cancellation and deadlines during the operation. +// - blockID: The identifier of the block for which transaction result error messages need to be processed. +// +// No errors are expected during normal operation. +func (e *Engine) handleTransactionResultErrorMessages(ctx context.Context, blockID flow.Identifier) error { + if e.transactionResultErrorMessages == nil { + return nil + } + + exists, err := e.transactionResultErrorMessages.Exists(blockID) + if err != nil { + return fmt.Errorf("could not check existance of transaction result error messages: %w", err) + } + + if exists { + return nil + } + + // retrieves error messages from the backend if they do not already exist in storage + execNodes, err := e.execNodeIdentitiesProvider.ExecutionNodesForBlockID( + ctx, + blockID, + ) + if err != nil { + e.log.Error().Err(err).Msg(fmt.Sprintf("failed to found execution nodes for block id: %s", blockID)) + + return fmt.Errorf("could not found execution nodes for block: %w", err) + } + + req := &execproto.GetTransactionErrorMessagesByBlockIDRequest{ + BlockId: convert.IdentifierToMessage(blockID), + } + + resp, execNode, err := e.backend.GetTransactionErrorMessagesFromAnyEN(ctx, execNodes, req) + if err != nil { + // continue, we will add functionality to backfill these later + e.log.Error().Err(err).Msg("failed to get transaction error messages from execution nodes") + return nil + } + + err = e.storeTransactionResultErrorMessages(blockID, resp, execNode) + if err != nil { + return fmt.Errorf("could not store error messages: %w", err) + } + + return nil +} + +// storeTransactionResultErrorMessages stores the transaction result error messages for a given block ID. +// +// Parameters: +// - blockID: The identifier of the block for which the error messages are to be stored. +// - errorMessagesResponses: A slice of responses containing the error messages to be stored. +// - execNode: The execution node associated with the error messages. +// +// No errors are expected during normal operation. +func (e *Engine) storeTransactionResultErrorMessages( + blockID flow.Identifier, + errorMessagesResponses []*execproto.GetTransactionErrorMessagesResponse_Result, + execNode *flow.IdentitySkeleton, +) error { + errorMessages := make([]flow.TransactionResultErrorMessage, 0, len(errorMessagesResponses)) + for _, value := range errorMessagesResponses { + errorMessage := flow.TransactionResultErrorMessage{ + ErrorMessage: value.ErrorMessage, + TransactionID: convert.MessageToIdentifier(value.TransactionId), + Index: value.Index, + ExecutorID: execNode.NodeID, + } + errorMessages = append(errorMessages, errorMessage) + } + + err := e.transactionResultErrorMessages.Store(blockID, errorMessages) + if err != nil { + return fmt.Errorf("failed to store transaction error messages: %w", err) + } + + return nil +} + // process processes the given ingestion engine event. Events that are given // to this function originate within the expulsion engine on the node with the // given origin ID. @@ -380,7 +514,7 @@ func (e *Engine) OnFinalizedBlock(*model.Block) { // - storage.ErrAlreadyExists - if the collection within block or an execution result ID already exists in the database. // - generic error in case of unexpected failure from the database layer, or failure // to decode an existing database value. -func (e *Engine) processFinalizedBlock(block *flow.Block) error { +func (e *Engine) processFinalizedBlock(ctx context.Context, block *flow.Block) error { // FIX: we can't index guarantees here, as we might have more than one block // with the same collection as long as it is not finalized @@ -398,6 +532,11 @@ func (e *Engine) processFinalizedBlock(block *flow.Block) error { if err != nil { return fmt.Errorf("could not index block for execution result: %w", err) } + + err = e.handleTransactionResultErrorMessages(ctx, seal.BlockID) + if err != nil { + return err + } } // skip requesting collections, if this block is below the last full block height diff --git a/engine/access/ingestion/engine_test.go b/engine/access/ingestion/engine_test.go index d93056bd80c..9e912d0ef04 100644 --- a/engine/access/ingestion/engine_test.go +++ b/engine/access/ingestion/engine_test.go @@ -2,6 +2,7 @@ package ingestion import ( "context" + "fmt" "math/rand" "os" "sync" @@ -9,12 +10,17 @@ import ( "time" "github.com/dgraph-io/badger/v2" + execproto "github.com/onflow/flow/protobuf/go/flow/execution" "github.com/rs/zerolog" "github.com/stretchr/testify/mock" "github.com/stretchr/testify/require" "github.com/stretchr/testify/suite" hotmodel "github.com/onflow/flow-go/consensus/hotstuff/model" + accessmock "github.com/onflow/flow-go/engine/access/mock" + "github.com/onflow/flow-go/engine/access/rpc/backend" + connectionmock "github.com/onflow/flow-go/engine/access/rpc/connection/mock" + commonrpc "github.com/onflow/flow-go/engine/common/rpc" "github.com/onflow/flow-go/model/flow" "github.com/onflow/flow-go/model/flow/filter" "github.com/onflow/flow-go/module" @@ -37,6 +43,8 @@ import ( "github.com/onflow/flow-go/utils/unittest/mocks" ) +const expectedErrorMsg = "expected test error" + type Suite struct { suite.Suite @@ -47,25 +55,29 @@ type Suite struct { params *protocol.Params } - me *modulemock.Local - net *mocknetwork.Network - request *modulemock.Requester - obsIdentity *flow.Identity - provider *mocknetwork.Engine - blocks *storage.Blocks - headers *storage.Headers - collections *storage.Collections - transactions *storage.Transactions - receipts *storage.ExecutionReceipts - results *storage.ExecutionResults - seals *storage.Seals - conduit *mocknetwork.Conduit - downloader *downloadermock.Downloader - sealedBlock *flow.Header - finalizedBlock *flow.Header - log zerolog.Logger - blockMap map[uint64]*flow.Block - rootBlock flow.Block + me *modulemock.Local + net *mocknetwork.Network + request *modulemock.Requester + obsIdentity *flow.Identity + provider *mocknetwork.Engine + blocks *storage.Blocks + headers *storage.Headers + collections *storage.Collections + transactions *storage.Transactions + receipts *storage.ExecutionReceipts + results *storage.ExecutionResults + seals *storage.Seals + txErrorMessages *storage.TransactionResultErrorMessages + conduit *mocknetwork.Conduit + downloader *downloadermock.Downloader + sealedBlock *flow.Header + finalizedBlock *flow.Header + log zerolog.Logger + blockMap map[uint64]*flow.Block + rootBlock flow.Block + + enNodeIDs flow.IdentifierList + backend *backend.Backend collectionExecutedMetric *indexer.CollectionExecutedMetricImpl @@ -75,6 +87,8 @@ type Suite struct { db *badger.DB dbDir string lastFullBlockHeight *counters.PersistentStrictMonotonicCounter + + execNodeIdentitiesProvider *commonrpc.ExecutionNodeIdentitiesProvider } func TestIngestEngine(t *testing.T) { @@ -127,6 +141,7 @@ func (s *Suite) SetupTest() { s.receipts = new(storage.ExecutionReceipts) s.transactions = new(storage.Transactions) s.results = new(storage.ExecutionResults) + s.txErrorMessages = storage.NewTransactionResultErrorMessages(s.T()) collectionsToMarkFinalized, err := stdmap.NewTimes(100) require.NoError(s.T(), err) collectionsToMarkExecuted, err := stdmap.NewTimes(100) @@ -176,6 +191,14 @@ func (s *Suite) SetupTest() { s.blocks, ) require.NoError(s.T(), err) + + s.execNodeIdentitiesProvider = commonrpc.NewExecutionNodeIdentitiesProvider( + s.log, + s.proto.state, + s.receipts, + nil, + s.enNodeIDs, + ) } // initIngestionEngine create new instance of ingestion engine and waits when it starts @@ -189,7 +212,26 @@ func (s *Suite) initIngestionEngine(ctx irrecoverable.SignalerContext) *Engine { ) require.NoError(s.T(), err) - eng, err := New(s.log, s.net, s.proto.state, s.me, s.request, s.blocks, s.headers, s.collections, s.transactions, s.results, s.receipts, s.collectionExecutedMetric, processedHeight, s.lastFullBlockHeight) + eng, err := New( + s.log, + s.net, + s.proto.state, + s.me, + s.request, + s.blocks, + s.headers, + s.collections, + s.transactions, + s.results, + s.receipts, + s.txErrorMessages, + s.collectionExecutedMetric, + processedHeight, + s.lastFullBlockHeight, + s.backend, + s.execNodeIdentitiesProvider, + ) + require.NoError(s.T(), err) eng.ComponentManager.Start(ctx) @@ -264,6 +306,10 @@ func (s *Suite) TestOnFinalizedBlockSingle() { s.blocks.On("IndexBlockForCollections", block.ID(), []flow.Identifier(flow.GetIDs(block.Payload.Guarantees))).Return(nil).Once() for _, seal := range block.Payload.Seals { s.results.On("Index", seal.BlockID, seal.ResultID).Return(nil).Once() + + // Mock the txErrorMessages storage to confirm that error messages exist. + s.txErrorMessages.On("Exists", seal.BlockID). + Return(true, nil).Once() } missingCollectionCount := 4 @@ -344,6 +390,9 @@ func (s *Suite) TestOnFinalizedBlockSeveralBlocksAhead() { } for _, seal := range block.Payload.Seals { s.results.On("Index", seal.BlockID, seal.ResultID).Return(nil).Once() + // Mock the txErrorMessages storage to confirm that error messages exist. + s.txErrorMessages.On("Exists", seal.BlockID). + Return(true, nil).Once() } } @@ -791,6 +840,139 @@ func (s *Suite) TestProcessBackgroundCalls() { }) } +// TestTransactionResultErrorMessagesAreFetched checks that transaction result error messages +// are properly fetched from the execution nodes, processed, and stored in the protocol database. +func (s *Suite) TestTransactionResultErrorMessagesAreFetched() { + irrecoverableCtx := irrecoverable.NewMockSignalerContext(s.T(), s.ctx) + + block := unittest.BlockFixture() + blockId := block.ID() + + // Create a mock execution client to simulate communication with execution nodes. + execClient := new(accessmock.ExecutionAPIClient) + + // Create identities for 1 execution nodes. + enIdentities := unittest.IdentityListFixture(1, unittest.WithRole(flow.RoleExecution)) + s.enNodeIDs = enIdentities.NodeIDs() + + // create a mock connection factory + connFactory := connectionmock.NewConnectionFactory(s.T()) + connFactory.On("GetExecutionAPIClient", mock.Anything).Return(execClient, &mocks.MockCloser{}, nil) + + // Mock the protocol snapshot to return fixed execution node IDs. + _, fixedENIDs := s.setupReceipts(&block) + s.proto.snapshot.On("Identities", mock.Anything).Return(fixedENIDs, nil) + s.proto.state.On("AtBlockID", blockId).Return(s.proto.snapshot) + + // Mock the finalized root block header with height 0. + header := unittest.BlockHeaderFixture(unittest.WithHeaderHeight(0)) + s.proto.params.On("FinalizedRoot").Return(header, nil) + + // Initialize the backend with the mocked state, blocks, headers, transactions, etc. + var err error + s.backend, err = backend.New(backend.Params{ + State: s.proto.state, + Blocks: s.blocks, + Headers: s.headers, + Transactions: s.transactions, + ExecutionReceipts: s.receipts, + ExecutionResults: s.results, + ConnFactory: connFactory, + MaxHeightRange: backend.DefaultMaxHeightRange, + Log: s.log, + SnapshotHistoryLimit: backend.DefaultSnapshotHistoryLimit, + Communicator: backend.NewNodeCommunicator(false), + ScriptExecutionMode: backend.IndexQueryModeExecutionNodesOnly, + TxResultQueryMode: backend.IndexQueryModeExecutionNodesOnly, + ChainID: flow.Testnet, + ExecNodeIdentitiesProvider: s.execNodeIdentitiesProvider, + }) + require.NoError(s.T(), err) + + // Initialize the ingestion engine with the irrecoverable context. + eng := s.initIngestionEngine(irrecoverableCtx) + + // Create mock transaction results with a mix of failed and non-failed transactions. + resultsByBlockID := make([]flow.LightTransactionResult, 0) + for i := 0; i < 5; i++ { + resultsByBlockID = append(resultsByBlockID, flow.LightTransactionResult{ + TransactionID: unittest.IdentifierFixture(), + Failed: i%2 == 0, // create a mix of failed and non-failed transactions + ComputationUsed: 0, + }) + } + + // Prepare a request to fetch transaction error messages by block ID from execution nodes. + exeEventReq := &execproto.GetTransactionErrorMessagesByBlockIDRequest{ + BlockId: blockId[:], + } + exeErrMessagesResp := &execproto.GetTransactionErrorMessagesResponse{} + + // Prepare the expected transaction error messages that should be stored. + var expectedStoreTxErrorMessages []flow.TransactionResultErrorMessage + + for i, result := range resultsByBlockID { + if result.Failed { + errMsg := fmt.Sprintf("%s.%s", expectedErrorMsg, result.TransactionID) + exeErrMessagesResp.Results = append(exeErrMessagesResp.Results, &execproto.GetTransactionErrorMessagesResponse_Result{ + TransactionId: result.TransactionID[:], + ErrorMessage: errMsg, + Index: uint32(i), + }) + + expectedStoreTxErrorMessages = append(expectedStoreTxErrorMessages, + flow.TransactionResultErrorMessage{ + TransactionID: result.TransactionID, + ErrorMessage: errMsg, + Index: uint32(i), + ExecutorID: fixedENIDs.NodeIDs()[0], + }) + } + } + execClient.On("GetTransactionErrorMessagesByBlockID", mock.Anything, exeEventReq). + Return(exeErrMessagesResp, nil). + Once() + + // Mock the txErrorMessages storage to confirm that error messages do not exist yet. + s.txErrorMessages.On("Exists", blockId). + Return(false, nil).Once() + + // Mock the storage of the fetched error messages into the protocol database. + s.txErrorMessages.On("Store", blockId, expectedStoreTxErrorMessages). + Return(nil).Once() + + err = eng.handleTransactionResultErrorMessages(irrecoverableCtx, blockId) + require.NoError(s.T(), err) + + // Verify that the mock expectations for storing the error messages were met. + s.txErrorMessages.AssertExpectations(s.T()) + + // Now simulate the second try when the error messages already exist in storage. + // Mock the txErrorMessages storage to confirm that error messages exist. + s.txErrorMessages.On("Exists", blockId). + Return(true, nil).Once() + err = eng.handleTransactionResultErrorMessages(irrecoverableCtx, blockId) + require.NoError(s.T(), err) + + // Verify that the mock expectations for storing the error messages were not met. + s.txErrorMessages.AssertExpectations(s.T()) +} + +// setupReceipts sets up mock execution receipts for a block and returns the receipts along +// with the identities of the execution nodes that processed them. +func (s *Suite) setupReceipts(block *flow.Block) ([]*flow.ExecutionReceipt, flow.IdentityList) { + ids := unittest.IdentityListFixture(1, unittest.WithRole(flow.RoleExecution)) + receipt1 := unittest.ReceiptForBlockFixture(block) + receipt1.ExecutorID = ids[0].NodeID + + receipts := flow.ExecutionReceiptList{receipt1} + s.receipts. + On("ByBlockID", block.ID()). + Return(receipts, nil) + + return receipts, ids +} + func (s *Suite) TestComponentShutdown() { irrecoverableCtx := irrecoverable.NewMockSignalerContext(s.T(), s.ctx) eng := s.initIngestionEngine(irrecoverableCtx) diff --git a/engine/access/rpc/backend/backend.go b/engine/access/rpc/backend/backend.go index a74e7a1851d..cbb26bf4d08 100644 --- a/engine/access/rpc/backend/backend.go +++ b/engine/access/rpc/backend/backend.go @@ -16,10 +16,10 @@ import ( "github.com/onflow/flow-go/engine/access/rpc/connection" "github.com/onflow/flow-go/engine/access/subscription" "github.com/onflow/flow-go/engine/common/rpc" + commonrpc "github.com/onflow/flow-go/engine/common/rpc" "github.com/onflow/flow-go/engine/common/version" "github.com/onflow/flow-go/fvm/blueprints" "github.com/onflow/flow-go/model/flow" - "github.com/onflow/flow-go/model/flow/filter" "github.com/onflow/flow-go/module" "github.com/onflow/flow-go/module/counters" "github.com/onflow/flow-go/module/execution" @@ -28,12 +28,6 @@ import ( "github.com/onflow/flow-go/storage" ) -// minExecutionNodesCnt is the minimum number of execution nodes expected to have sent the execution receipt for a block -const minExecutionNodesCnt = 2 - -// maxAttemptsForExecutionReceipt is the maximum number of attempts to find execution receipts for a given block ID -const maxAttemptsForExecutionReceipt = 3 - // DefaultMaxHeightRange is the default maximum size of range requests. const DefaultMaxHeightRange = 250 @@ -48,11 +42,6 @@ const DefaultLoggedScriptsCacheSize = 1_000_000 // DefaultConnectionPoolSize is the default size for the connection pool to collection and execution nodes const DefaultConnectionPoolSize = 250 -var ( - preferredENIdentifiers flow.IdentifierList - fixedENIdentifiers flow.IdentifierList -) - // Backend implements the Access API. // // It is composed of several sub-backends that implement part of the Access API. @@ -89,40 +78,39 @@ type Backend struct { } type Params struct { - State protocol.State - CollectionRPC accessproto.AccessAPIClient - HistoricalAccessNodes []accessproto.AccessAPIClient - Blocks storage.Blocks - Headers storage.Headers - Collections storage.Collections - Transactions storage.Transactions - ExecutionReceipts storage.ExecutionReceipts - ExecutionResults storage.ExecutionResults - ChainID flow.ChainID - AccessMetrics module.AccessMetrics - ConnFactory connection.ConnectionFactory - RetryEnabled bool - MaxHeightRange uint - PreferredExecutionNodeIDs []string - FixedExecutionNodeIDs []string - Log zerolog.Logger - SnapshotHistoryLimit int - Communicator Communicator - TxResultCacheSize uint - TxErrorMessagesCacheSize uint - ScriptExecutor execution.ScriptExecutor - ScriptExecutionMode IndexQueryMode - CheckPayerBalanceMode access.PayerBalanceMode - EventQueryMode IndexQueryMode - BlockTracker subscription.BlockTracker - SubscriptionHandler *subscription.SubscriptionHandler - - EventsIndex *index.EventsIndex - TxResultQueryMode IndexQueryMode - TxResultsIndex *index.TransactionResultsIndex - LastFullBlockHeight *counters.PersistentStrictMonotonicCounter - IndexReporter state_synchronization.IndexReporter - VersionControl *version.VersionControl + State protocol.State + CollectionRPC accessproto.AccessAPIClient + HistoricalAccessNodes []accessproto.AccessAPIClient + Blocks storage.Blocks + Headers storage.Headers + Collections storage.Collections + Transactions storage.Transactions + ExecutionReceipts storage.ExecutionReceipts + ExecutionResults storage.ExecutionResults + TxResultErrorMessages storage.TransactionResultErrorMessages + ChainID flow.ChainID + AccessMetrics module.AccessMetrics + ConnFactory connection.ConnectionFactory + RetryEnabled bool + MaxHeightRange uint + Log zerolog.Logger + SnapshotHistoryLimit int + Communicator Communicator + TxResultCacheSize uint + ScriptExecutor execution.ScriptExecutor + ScriptExecutionMode IndexQueryMode + CheckPayerBalanceMode access.PayerBalanceMode + EventQueryMode IndexQueryMode + BlockTracker subscription.BlockTracker + SubscriptionHandler *subscription.SubscriptionHandler + + EventsIndex *index.EventsIndex + TxResultQueryMode IndexQueryMode + TxResultsIndex *index.TransactionResultsIndex + LastFullBlockHeight *counters.PersistentStrictMonotonicCounter + IndexReporter state_synchronization.IndexReporter + VersionControl *version.VersionControl + ExecNodeIdentitiesProvider *commonrpc.ExecutionNodeIdentitiesProvider } var _ TransactionErrorMessage = (*Backend)(nil) @@ -147,18 +135,6 @@ func New(params Params) (*Backend, error) { } } - // NOTE: The transaction error message cache is currently only used by the access node and not by the observer node. - // To avoid introducing unnecessary command line arguments in the observer, one case could be that the error - // message cache is nil for the observer node. - var txErrorMessagesCache *lru.Cache[flow.Identifier, string] - - if params.TxErrorMessagesCacheSize > 0 { - txErrorMessagesCache, err = lru.New[flow.Identifier, string](int(params.TxErrorMessagesCacheSize)) - if err != nil { - return nil, fmt.Errorf("failed to init cache for transaction error messages: %w", err) - } - } - // the system tx is hardcoded and never changes during runtime systemTx, err := blueprints.SystemChunkTransaction(params.ChainID.Chain()) if err != nil { @@ -181,28 +157,28 @@ func New(params Params) (*Backend, error) { BlockTracker: params.BlockTracker, // create the sub-backends backendScripts: backendScripts{ - log: params.Log, - headers: params.Headers, - executionReceipts: params.ExecutionReceipts, - connFactory: params.ConnFactory, - state: params.State, - metrics: params.AccessMetrics, - loggedScripts: loggedScripts, - nodeCommunicator: params.Communicator, - scriptExecutor: params.ScriptExecutor, - scriptExecMode: params.ScriptExecutionMode, + log: params.Log, + headers: params.Headers, + connFactory: params.ConnFactory, + state: params.State, + metrics: params.AccessMetrics, + loggedScripts: loggedScripts, + nodeCommunicator: params.Communicator, + scriptExecutor: params.ScriptExecutor, + scriptExecMode: params.ScriptExecutionMode, + execNodeIdentitiesProvider: params.ExecNodeIdentitiesProvider, }, backendEvents: backendEvents{ - log: params.Log, - chain: params.ChainID.Chain(), - state: params.State, - headers: params.Headers, - executionReceipts: params.ExecutionReceipts, - connFactory: params.ConnFactory, - maxHeightRange: params.MaxHeightRange, - nodeCommunicator: params.Communicator, - queryMode: params.EventQueryMode, - eventsIndex: params.EventsIndex, + log: params.Log, + chain: params.ChainID.Chain(), + state: params.State, + headers: params.Headers, + connFactory: params.ConnFactory, + maxHeightRange: params.MaxHeightRange, + nodeCommunicator: params.Communicator, + queryMode: params.EventQueryMode, + eventsIndex: params.EventsIndex, + execNodeIdentitiesProvider: params.ExecNodeIdentitiesProvider, }, backendBlockHeaders: backendBlockHeaders{ headers: params.Headers, @@ -213,14 +189,14 @@ func New(params Params) (*Backend, error) { state: params.State, }, backendAccounts: backendAccounts{ - log: params.Log, - state: params.State, - headers: params.Headers, - executionReceipts: params.ExecutionReceipts, - connFactory: params.ConnFactory, - nodeCommunicator: params.Communicator, - scriptExecutor: params.ScriptExecutor, - scriptExecMode: params.ScriptExecutionMode, + log: params.Log, + state: params.State, + headers: params.Headers, + connFactory: params.ConnFactory, + nodeCommunicator: params.Communicator, + scriptExecutor: params.ScriptExecutor, + scriptExecMode: params.ScriptExecutionMode, + execNodeIdentitiesProvider: params.ExecNodeIdentitiesProvider, }, backendExecutionResults: backendExecutionResults{ executionResults: params.ExecutionResults, @@ -259,7 +235,7 @@ func New(params Params) (*Backend, error) { staticCollectionRPC: params.CollectionRPC, chainID: params.ChainID, transactions: params.Transactions, - executionReceipts: params.ExecutionReceipts, + txResultErrorMessages: params.TxResultErrorMessages, transactionValidator: txValidator, transactionMetrics: params.AccessMetrics, retry: retry, @@ -267,10 +243,10 @@ func New(params Params) (*Backend, error) { previousAccessNodes: params.HistoricalAccessNodes, nodeCommunicator: params.Communicator, txResultCache: txResCache, - txErrorMessagesCache: txErrorMessagesCache, txResultQueryMode: params.TxResultQueryMode, systemTx: systemTx, systemTxID: systemTxID, + execNodeIdentitiesProvider: params.ExecNodeIdentitiesProvider, } // TODO: The TransactionErrorMessage interface should be reorganized in future, as it is implemented in backendTransactions but used in TransactionsLocalDataProvider, and its initialization is somewhat quirky. @@ -287,31 +263,9 @@ func New(params Params) (*Backend, error) { retry.SetBackend(b) - preferredENIdentifiers, err = identifierList(params.PreferredExecutionNodeIDs) - if err != nil { - return nil, fmt.Errorf("failed to convert node id string to Flow Identifier for preferred EN map: %w", err) - } - - fixedENIdentifiers, err = identifierList(params.FixedExecutionNodeIDs) - if err != nil { - return nil, fmt.Errorf("failed to convert node id string to Flow Identifier for fixed EN map: %w", err) - } - return b, nil } -func identifierList(ids []string) (flow.IdentifierList, error) { - idList := make(flow.IdentifierList, len(ids)) - for i, idStr := range ids { - id, err := flow.HexStringToIdentifier(idStr) - if err != nil { - return nil, fmt.Errorf("failed to convert node id string %s to Flow Identifier: %w", id, err) - } - idList[i] = id - } - return idList, nil -} - func configureTransactionValidator( state protocol.State, chainID flow.ChainID, @@ -418,226 +372,3 @@ func (b *Backend) GetNetworkParameters(_ context.Context) access.NetworkParamete ChainID: b.chainID, } } - -// executionNodesForBlockID returns upto maxNodesCnt number of randomly chosen execution node identities -// which have executed the given block ID. -// If no such execution node is found, an InsufficientExecutionReceipts error is returned. -func executionNodesForBlockID( - ctx context.Context, - blockID flow.Identifier, - executionReceipts storage.ExecutionReceipts, - state protocol.State, - log zerolog.Logger, -) (flow.IdentitySkeletonList, error) { - var ( - executorIDs flow.IdentifierList - err error - ) - - // check if the block ID is of the root block. If it is then don't look for execution receipts since they - // will not be present for the root block. - rootBlock := state.Params().FinalizedRoot() - - if rootBlock.ID() == blockID { - executorIdentities, err := state.Final().Identities(filter.HasRole[flow.Identity](flow.RoleExecution)) - if err != nil { - return nil, fmt.Errorf("failed to retreive execution IDs for block ID %v: %w", blockID, err) - } - executorIDs = executorIdentities.NodeIDs() - } else { - // try to find at least minExecutionNodesCnt execution node ids from the execution receipts for the given blockID - for attempt := 0; attempt < maxAttemptsForExecutionReceipt; attempt++ { - executorIDs, err = findAllExecutionNodes(blockID, executionReceipts, log) - if err != nil { - return nil, err - } - - if len(executorIDs) >= minExecutionNodesCnt { - break - } - - // log the attempt - log.Debug().Int("attempt", attempt).Int("max_attempt", maxAttemptsForExecutionReceipt). - Int("execution_receipts_found", len(executorIDs)). - Str("block_id", blockID.String()). - Msg("insufficient execution receipts") - - // if one or less execution receipts may have been received then re-query - // in the hope that more might have been received by now - - select { - case <-ctx.Done(): - return nil, ctx.Err() - case <-time.After(100 * time.Millisecond << time.Duration(attempt)): - // retry after an exponential backoff - } - } - - receiptCnt := len(executorIDs) - // if less than minExecutionNodesCnt execution receipts have been received so far, then return random ENs - if receiptCnt < minExecutionNodesCnt { - newExecutorIDs, err := state.AtBlockID(blockID).Identities(filter.HasRole[flow.Identity](flow.RoleExecution)) - if err != nil { - return nil, fmt.Errorf("failed to retreive execution IDs for block ID %v: %w", blockID, err) - } - executorIDs = newExecutorIDs.NodeIDs() - } - } - - // choose from the preferred or fixed execution nodes - subsetENs, err := chooseExecutionNodes(state, executorIDs) - if err != nil { - return nil, fmt.Errorf("failed to retreive execution IDs for block ID %v: %w", blockID, err) - } - - if len(subsetENs) == 0 { - return nil, fmt.Errorf("no matching execution node found for block ID %v", blockID) - } - - return subsetENs, nil -} - -// findAllExecutionNodes find all the execution nodes ids from the execution receipts that have been received for the -// given blockID -func findAllExecutionNodes( - blockID flow.Identifier, - executionReceipts storage.ExecutionReceipts, - log zerolog.Logger, -) (flow.IdentifierList, error) { - // lookup the receipt's storage with the block ID - allReceipts, err := executionReceipts.ByBlockID(blockID) - if err != nil { - return nil, fmt.Errorf("failed to retreive execution receipts for block ID %v: %w", blockID, err) - } - - executionResultMetaList := make(flow.ExecutionReceiptMetaList, 0, len(allReceipts)) - for _, r := range allReceipts { - executionResultMetaList = append(executionResultMetaList, r.Meta()) - } - executionResultGroupedMetaList := executionResultMetaList.GroupByResultID() - - // maximum number of matching receipts found so far for any execution result id - maxMatchedReceiptCnt := 0 - // execution result id key for the highest number of matching receipts in the identicalReceipts map - var maxMatchedReceiptResultID flow.Identifier - - // find the largest list of receipts which have the same result ID - for resultID, executionReceiptList := range executionResultGroupedMetaList { - currentMatchedReceiptCnt := executionReceiptList.Size() - if currentMatchedReceiptCnt > maxMatchedReceiptCnt { - maxMatchedReceiptCnt = currentMatchedReceiptCnt - maxMatchedReceiptResultID = resultID - } - } - - // if there are more than one execution result for the same block ID, log as error - if executionResultGroupedMetaList.NumberGroups() > 1 { - identicalReceiptsStr := fmt.Sprintf("%v", flow.GetIDs(allReceipts)) - log.Error(). - Str("block_id", blockID.String()). - Str("execution_receipts", identicalReceiptsStr). - Msg("execution receipt mismatch") - } - - // pick the largest list of matching receipts - matchingReceiptMetaList := executionResultGroupedMetaList.GetGroup(maxMatchedReceiptResultID) - - metaReceiptGroupedByExecutorID := matchingReceiptMetaList.GroupByExecutorID() - - // collect all unique execution node ids from the receipts - var executorIDs flow.IdentifierList - for executorID := range metaReceiptGroupedByExecutorID { - executorIDs = append(executorIDs, executorID) - } - - return executorIDs, nil -} - -// chooseExecutionNodes finds the subset of execution nodes defined in the identity table by first -// choosing the preferred execution nodes which have executed the transaction. If no such preferred -// execution nodes are found, then the fixed execution nodes defined in the identity table are returned -// If neither preferred nor fixed nodes are defined, then all execution node matching the executor IDs are returned. -// e.g. If execution nodes in identity table are {1,2,3,4}, preferred ENs are defined as {2,3,4} -// and the executor IDs is {1,2,3}, then {2, 3} is returned as the chosen subset of ENs -func chooseExecutionNodes(state protocol.State, executorIDs flow.IdentifierList) (flow.IdentitySkeletonList, error) { - allENs, err := state.Final().Identities(filter.HasRole[flow.Identity](flow.RoleExecution)) - if err != nil { - return nil, fmt.Errorf("failed to retrieve all execution IDs: %w", err) - } - - // choose from preferred EN IDs - if len(preferredENIdentifiers) > 0 { - chosenIDs := chooseFromPreferredENIDs(allENs, executorIDs) - return chosenIDs.ToSkeleton(), nil - } - - // if no preferred EN ID is found, then choose from the fixed EN IDs - if len(fixedENIdentifiers) > 0 { - // choose fixed ENs which have executed the transaction - chosenIDs := allENs.Filter(filter.And( - filter.HasNodeID[flow.Identity](fixedENIdentifiers...), - filter.HasNodeID[flow.Identity](executorIDs...), - )) - if len(chosenIDs) > 0 { - return chosenIDs.ToSkeleton(), nil - } - // if no such ENs are found, then just choose all fixed ENs - chosenIDs = allENs.Filter(filter.HasNodeID[flow.Identity](fixedENIdentifiers...)) - return chosenIDs.ToSkeleton(), nil - } - - // if no preferred or fixed ENs have been specified, then return all executor IDs i.e., no preference at all - return allENs.Filter(filter.HasNodeID[flow.Identity](executorIDs...)).ToSkeleton(), nil -} - -// chooseFromPreferredENIDs finds the subset of execution nodes if preferred execution nodes are defined. -// If preferredENIdentifiers is set and there are less than maxNodesCnt nodes selected, than the list is padded up to -// maxNodesCnt nodes using the following order: -// 1. Use any EN with a receipt. -// 2. Use any preferred node not already selected. -// 3. Use any EN not already selected. -func chooseFromPreferredENIDs(allENs flow.IdentityList, executorIDs flow.IdentifierList) flow.IdentityList { - var chosenIDs flow.IdentityList - - // filter for both preferred and executor IDs - chosenIDs = allENs.Filter(filter.And( - filter.HasNodeID[flow.Identity](preferredENIdentifiers...), - filter.HasNodeID[flow.Identity](executorIDs...), - )) - - if len(chosenIDs) >= maxNodesCnt { - return chosenIDs - } - - // function to add nodes to chosenIDs if they are not already included - addIfNotExists := func(candidates flow.IdentityList) { - for _, en := range candidates { - _, exists := chosenIDs.ByNodeID(en.NodeID) - if !exists { - chosenIDs = append(chosenIDs, en) - if len(chosenIDs) >= maxNodesCnt { - return - } - } - } - } - - // add any EN with a receipt - receiptENs := allENs.Filter(filter.HasNodeID[flow.Identity](executorIDs...)) - addIfNotExists(receiptENs) - if len(chosenIDs) >= maxNodesCnt { - return chosenIDs - } - - // add any preferred node not already selected - preferredENs := allENs.Filter(filter.HasNodeID[flow.Identity](preferredENIdentifiers...)) - addIfNotExists(preferredENs) - if len(chosenIDs) >= maxNodesCnt { - return chosenIDs - } - - // add any EN not already selected - addIfNotExists(allENs) - - return chosenIDs -} diff --git a/engine/access/rpc/backend/backend_accounts.go b/engine/access/rpc/backend/backend_accounts.go index f0d16237c25..ca52455f7ba 100644 --- a/engine/access/rpc/backend/backend_accounts.go +++ b/engine/access/rpc/backend/backend_accounts.go @@ -14,6 +14,7 @@ import ( "github.com/onflow/flow-go/engine/access/rpc/connection" "github.com/onflow/flow-go/engine/common/rpc" + commonrpc "github.com/onflow/flow-go/engine/common/rpc" "github.com/onflow/flow-go/engine/common/rpc/convert" fvmerrors "github.com/onflow/flow-go/fvm/errors" "github.com/onflow/flow-go/model/flow" @@ -24,14 +25,14 @@ import ( ) type backendAccounts struct { - log zerolog.Logger - state protocol.State - headers storage.Headers - executionReceipts storage.ExecutionReceipts - connFactory connection.ConnectionFactory - nodeCommunicator Communicator - scriptExecutor execution.ScriptExecutor - scriptExecMode IndexQueryMode + log zerolog.Logger + state protocol.State + headers storage.Headers + connFactory connection.ConnectionFactory + nodeCommunicator Communicator + scriptExecutor execution.ScriptExecutor + scriptExecMode IndexQueryMode + execNodeIdentitiesProvider *commonrpc.ExecutionNodeIdentitiesProvider } // GetAccount returns the account details at the latest sealed block. @@ -419,7 +420,10 @@ func (b *backendAccounts) getAccountFromAnyExeNode( BlockId: blockID[:], } - execNodes, err := executionNodesForBlockID(ctx, blockID, b.executionReceipts, b.state, b.log) + execNodes, err := b.execNodeIdentitiesProvider.ExecutionNodesForBlockID( + ctx, + blockID, + ) if err != nil { return nil, rpc.ConvertError(err, "failed to find execution node to query", codes.Internal) } diff --git a/engine/access/rpc/backend/backend_accounts_test.go b/engine/access/rpc/backend/backend_accounts_test.go index 50ca272da03..1311b0330d5 100644 --- a/engine/access/rpc/backend/backend_accounts_test.go +++ b/engine/access/rpc/backend/backend_accounts_test.go @@ -13,6 +13,7 @@ import ( access "github.com/onflow/flow-go/engine/access/mock" connectionmock "github.com/onflow/flow-go/engine/access/rpc/connection/mock" + commonrpc "github.com/onflow/flow-go/engine/common/rpc" "github.com/onflow/flow-go/engine/common/rpc/convert" "github.com/onflow/flow-go/model/flow" execmock "github.com/onflow/flow-go/module/execution/mock" @@ -21,6 +22,7 @@ import ( "github.com/onflow/flow-go/storage" storagemock "github.com/onflow/flow-go/storage/mock" "github.com/onflow/flow-go/utils/unittest" + "github.com/onflow/flow-go/utils/unittest/mocks" execproto "github.com/onflow/flow/protobuf/go/flow/execution" ) @@ -77,12 +79,18 @@ func (s *BackendAccountsSuite) SetupTest() { func (s *BackendAccountsSuite) defaultBackend() *backendAccounts { return &backendAccounts{ - log: s.log, - state: s.state, - headers: s.headers, - executionReceipts: s.receipts, - connFactory: s.connectionFactory, - nodeCommunicator: NewNodeCommunicator(false), + log: s.log, + state: s.state, + headers: s.headers, + connFactory: s.connectionFactory, + nodeCommunicator: NewNodeCommunicator(false), + execNodeIdentitiesProvider: commonrpc.NewExecutionNodeIdentitiesProvider( + s.log, + s.state, + s.receipts, + flow.IdentifierList{}, + flow.IdentifierList{}, + ), } } @@ -100,7 +108,7 @@ func (s *BackendAccountsSuite) setupExecutionNodes(block *flow.Block) { s.receipts.On("ByBlockID", block.ID()).Return(receipts, nil) s.connectionFactory.On("GetExecutionAPIClient", mock.Anything). - Return(s.execClient, &mockCloser{}, nil) + Return(s.execClient, &mocks.MockCloser{}, nil) } // setupENSuccessResponse configures the execution node client to return a successful response diff --git a/engine/access/rpc/backend/backend_events.go b/engine/access/rpc/backend/backend_events.go index 2928e22aa7a..c3b872b0ad1 100644 --- a/engine/access/rpc/backend/backend_events.go +++ b/engine/access/rpc/backend/backend_events.go @@ -28,16 +28,16 @@ import ( ) type backendEvents struct { - headers storage.Headers - executionReceipts storage.ExecutionReceipts - state protocol.State - chain flow.Chain - connFactory connection.ConnectionFactory - log zerolog.Logger - maxHeightRange uint - nodeCommunicator Communicator - queryMode IndexQueryMode - eventsIndex *index.EventsIndex + headers storage.Headers + state protocol.State + chain flow.Chain + connFactory connection.ConnectionFactory + log zerolog.Logger + maxHeightRange uint + nodeCommunicator Communicator + queryMode IndexQueryMode + eventsIndex *index.EventsIndex + execNodeIdentitiesProvider *rpc.ExecutionNodeIdentitiesProvider } // blockMetadata is used to capture information about requested blocks to avoid repeated blockID @@ -303,7 +303,10 @@ func (b *backendEvents) getBlockEventsFromExecutionNode( // choose the last block ID to find the list of execution nodes lastBlockID := blockIDs[len(blockIDs)-1] - execNodes, err := executionNodesForBlockID(ctx, lastBlockID, b.executionReceipts, b.state, b.log) + execNodes, err := b.execNodeIdentitiesProvider.ExecutionNodesForBlockID( + ctx, + lastBlockID, + ) if err != nil { return nil, rpc.ConvertError(err, "failed to retrieve events from execution node", codes.Internal) } diff --git a/engine/access/rpc/backend/backend_events_test.go b/engine/access/rpc/backend/backend_events_test.go index b1b1a8c438d..a183c2bb0ab 100644 --- a/engine/access/rpc/backend/backend_events_test.go +++ b/engine/access/rpc/backend/backend_events_test.go @@ -21,6 +21,7 @@ import ( "github.com/onflow/flow-go/engine/access/index" access "github.com/onflow/flow-go/engine/access/mock" connectionmock "github.com/onflow/flow-go/engine/access/rpc/connection/mock" + commonrpc "github.com/onflow/flow-go/engine/common/rpc" "github.com/onflow/flow-go/engine/common/rpc/convert" "github.com/onflow/flow-go/model/flow" "github.com/onflow/flow-go/module/irrecoverable" @@ -30,6 +31,7 @@ import ( storagemock "github.com/onflow/flow-go/storage/mock" "github.com/onflow/flow-go/utils/unittest" "github.com/onflow/flow-go/utils/unittest/generator" + "github.com/onflow/flow-go/utils/unittest/mocks" ) var targetEvent string @@ -175,16 +177,22 @@ func (s *BackendEventsSuite) SetupTest() { func (s *BackendEventsSuite) defaultBackend() *backendEvents { return &backendEvents{ - log: s.log, - chain: s.chainID.Chain(), - state: s.state, - headers: s.headers, - executionReceipts: s.receipts, - connFactory: s.connectionFactory, - nodeCommunicator: NewNodeCommunicator(false), - maxHeightRange: DefaultMaxHeightRange, - queryMode: IndexQueryModeExecutionNodesOnly, - eventsIndex: s.eventsIndex, + log: s.log, + chain: s.chainID.Chain(), + state: s.state, + headers: s.headers, + connFactory: s.connectionFactory, + nodeCommunicator: NewNodeCommunicator(false), + maxHeightRange: DefaultMaxHeightRange, + queryMode: IndexQueryModeExecutionNodesOnly, + eventsIndex: s.eventsIndex, + execNodeIdentitiesProvider: commonrpc.NewExecutionNodeIdentitiesProvider( + s.log, + s.state, + s.receipts, + flow.IdentifierList{}, + flow.IdentifierList{}, + ), } } @@ -202,7 +210,7 @@ func (s *BackendEventsSuite) setupExecutionNodes(block *flow.Block) { s.receipts.On("ByBlockID", block.ID()).Return(receipts, nil) s.connectionFactory.On("GetExecutionAPIClient", mock.Anything). - Return(s.execClient, &mockCloser{}, nil) + Return(s.execClient, &mocks.MockCloser{}, nil) } // setupENSuccessResponse configures the execution node client to return a successful response diff --git a/engine/access/rpc/backend/backend_scripts.go b/engine/access/rpc/backend/backend_scripts.go index 2578245f02b..bea33fb5181 100644 --- a/engine/access/rpc/backend/backend_scripts.go +++ b/engine/access/rpc/backend/backend_scripts.go @@ -13,6 +13,7 @@ import ( "github.com/onflow/flow-go/engine/access/rpc/connection" "github.com/onflow/flow-go/engine/common/rpc" + commonrpc "github.com/onflow/flow-go/engine/common/rpc" fvmerrors "github.com/onflow/flow-go/fvm/errors" "github.com/onflow/flow-go/model/flow" "github.com/onflow/flow-go/module" @@ -27,16 +28,16 @@ import ( const uniqueScriptLoggingTimeWindow = 10 * time.Minute type backendScripts struct { - log zerolog.Logger - headers storage.Headers - executionReceipts storage.ExecutionReceipts - state protocol.State - connFactory connection.ConnectionFactory - metrics module.BackendScriptsMetrics - loggedScripts *lru.Cache[[md5.Size]byte, time.Time] - nodeCommunicator Communicator - scriptExecutor execution.ScriptExecutor - scriptExecMode IndexQueryMode + log zerolog.Logger + headers storage.Headers + state protocol.State + connFactory connection.ConnectionFactory + metrics module.BackendScriptsMetrics + loggedScripts *lru.Cache[[md5.Size]byte, time.Time] + nodeCommunicator Communicator + scriptExecutor execution.ScriptExecutor + scriptExecMode IndexQueryMode + execNodeIdentitiesProvider *commonrpc.ExecutionNodeIdentitiesProvider } // scriptExecutionRequest encapsulates the data needed to execute a script to make it easier @@ -224,7 +225,7 @@ func (b *backendScripts) executeScriptOnAvailableExecutionNodes( r *scriptExecutionRequest, ) ([]byte, time.Duration, error) { // find few execution nodes which have executed the block earlier and provided an execution receipt for it - executors, err := executionNodesForBlockID(ctx, r.blockID, b.executionReceipts, b.state, b.log) + executors, err := b.execNodeIdentitiesProvider.ExecutionNodesForBlockID(ctx, r.blockID) if err != nil { return nil, 0, status.Errorf(codes.Internal, "failed to find script executors at blockId %v: %v", r.blockID.String(), err) } diff --git a/engine/access/rpc/backend/backend_scripts_test.go b/engine/access/rpc/backend/backend_scripts_test.go index 398d9ac33ec..43abac07a62 100644 --- a/engine/access/rpc/backend/backend_scripts_test.go +++ b/engine/access/rpc/backend/backend_scripts_test.go @@ -18,6 +18,7 @@ import ( access "github.com/onflow/flow-go/engine/access/mock" connectionmock "github.com/onflow/flow-go/engine/access/rpc/connection/mock" + commonrpc "github.com/onflow/flow-go/engine/common/rpc" fvmerrors "github.com/onflow/flow-go/fvm/errors" "github.com/onflow/flow-go/model/flow" execmock "github.com/onflow/flow-go/module/execution/mock" @@ -27,6 +28,7 @@ import ( "github.com/onflow/flow-go/storage" storagemock "github.com/onflow/flow-go/storage/mock" "github.com/onflow/flow-go/utils/unittest" + "github.com/onflow/flow-go/utils/unittest/mocks" ) var ( @@ -96,14 +98,20 @@ func (s *BackendScriptsSuite) defaultBackend() *backendScripts { s.Require().NoError(err) return &backendScripts{ - log: s.log, - metrics: metrics.NewNoopCollector(), - state: s.state, - headers: s.headers, - executionReceipts: s.receipts, - loggedScripts: loggedScripts, - connFactory: s.connectionFactory, - nodeCommunicator: NewNodeCommunicator(false), + log: s.log, + metrics: metrics.NewNoopCollector(), + state: s.state, + headers: s.headers, + loggedScripts: loggedScripts, + connFactory: s.connectionFactory, + nodeCommunicator: NewNodeCommunicator(false), + execNodeIdentitiesProvider: commonrpc.NewExecutionNodeIdentitiesProvider( + s.log, + s.state, + s.receipts, + flow.IdentifierList{}, + flow.IdentifierList{}, + ), } } @@ -121,7 +129,7 @@ func (s *BackendScriptsSuite) setupExecutionNodes(block *flow.Block) { s.receipts.On("ByBlockID", block.ID()).Return(receipts, nil) s.connectionFactory.On("GetExecutionAPIClient", mock.Anything). - Return(s.execClient, &mockCloser{}, nil) + Return(s.execClient, &mocks.MockCloser{}, nil) } // setupENSuccessResponse configures the execution client mock to return a successful response diff --git a/engine/access/rpc/backend/backend_stream_blocks_test.go b/engine/access/rpc/backend/backend_stream_blocks_test.go index 69aa9e67823..6e655c8afda 100644 --- a/engine/access/rpc/backend/backend_stream_blocks_test.go +++ b/engine/access/rpc/backend/backend_stream_blocks_test.go @@ -148,15 +148,14 @@ func (s *BackendBlocksSuite) SetupTest() { // backendParams returns the Params configuration for the backend. func (s *BackendBlocksSuite) backendParams() Params { return Params{ - State: s.state, - Blocks: s.blocks, - Headers: s.headers, - ChainID: s.chainID, - MaxHeightRange: DefaultMaxHeightRange, - SnapshotHistoryLimit: DefaultSnapshotHistoryLimit, - AccessMetrics: metrics.NewNoopCollector(), - Log: s.log, - TxErrorMessagesCacheSize: 1000, + State: s.state, + Blocks: s.blocks, + Headers: s.headers, + ChainID: s.chainID, + MaxHeightRange: DefaultMaxHeightRange, + SnapshotHistoryLimit: DefaultSnapshotHistoryLimit, + AccessMetrics: metrics.NewNoopCollector(), + Log: s.log, SubscriptionHandler: subscription.NewSubscriptionHandler( s.log, s.broadcaster, diff --git a/engine/access/rpc/backend/backend_stream_transactions_test.go b/engine/access/rpc/backend/backend_stream_transactions_test.go index ec8ff353bf3..24cdf601f17 100644 --- a/engine/access/rpc/backend/backend_stream_transactions_test.go +++ b/engine/access/rpc/backend/backend_stream_transactions_test.go @@ -192,22 +192,21 @@ func (s *TransactionStatusSuite) TearDownTest() { // backendParams returns the Params configuration for the backend. func (s *TransactionStatusSuite) backendParams() Params { return Params{ - State: s.state, - Blocks: s.blocks, - Headers: s.headers, - Collections: s.collections, - Transactions: s.transactions, - ExecutionReceipts: s.receipts, - ExecutionResults: s.results, - ChainID: s.chainID, - CollectionRPC: s.colClient, - MaxHeightRange: DefaultMaxHeightRange, - SnapshotHistoryLimit: DefaultSnapshotHistoryLimit, - Communicator: NewNodeCommunicator(false), - AccessMetrics: metrics.NewNoopCollector(), - Log: s.log, - TxErrorMessagesCacheSize: 1000, - BlockTracker: s.blockTracker, + State: s.state, + Blocks: s.blocks, + Headers: s.headers, + Collections: s.collections, + Transactions: s.transactions, + ExecutionReceipts: s.receipts, + ExecutionResults: s.results, + ChainID: s.chainID, + CollectionRPC: s.colClient, + MaxHeightRange: DefaultMaxHeightRange, + SnapshotHistoryLimit: DefaultSnapshotHistoryLimit, + Communicator: NewNodeCommunicator(false), + AccessMetrics: metrics.NewNoopCollector(), + Log: s.log, + BlockTracker: s.blockTracker, SubscriptionHandler: subscription.NewSubscriptionHandler( s.log, s.broadcaster, diff --git a/engine/access/rpc/backend/backend_test.go b/engine/access/rpc/backend/backend_test.go index 4974cbe6ce7..93f3d24eeb1 100644 --- a/engine/access/rpc/backend/backend_test.go +++ b/engine/access/rpc/backend/backend_test.go @@ -30,6 +30,7 @@ import ( backendmock "github.com/onflow/flow-go/engine/access/rpc/backend/mock" "github.com/onflow/flow-go/engine/access/rpc/connection" connectionmock "github.com/onflow/flow-go/engine/access/rpc/connection/mock" + commonrpc "github.com/onflow/flow-go/engine/common/rpc" "github.com/onflow/flow-go/engine/common/rpc/convert" "github.com/onflow/flow-go/engine/common/version" "github.com/onflow/flow-go/fvm/blueprints" @@ -49,6 +50,7 @@ import ( storagemock "github.com/onflow/flow-go/storage/mock" "github.com/onflow/flow-go/utils/unittest" "github.com/onflow/flow-go/utils/unittest/generator" + "github.com/onflow/flow-go/utils/unittest/mocks" ) const TEST_MAX_HEIGHT = 100 @@ -73,6 +75,7 @@ type Suite struct { results *storagemock.ExecutionResults transactionResults *storagemock.LightTransactionResults events *storagemock.Events + txErrorMessages *storagemock.TransactionResultErrorMessages db *badger.DB dbDir string @@ -88,6 +91,9 @@ type Suite struct { chainID flow.ChainID systemTx *flow.TransactionBody + + fixedExecutionNodeIDs flow.IdentifierList + preferredExecutionNodeIDs flow.IdentifierList } func TestHandler(t *testing.T) { @@ -113,6 +119,7 @@ func (suite *Suite) SetupTest() { suite.collections = new(storagemock.Collections) suite.receipts = new(storagemock.ExecutionReceipts) suite.results = new(storagemock.ExecutionResults) + suite.txErrorMessages = storagemock.NewTransactionResultErrorMessages(suite.T()) suite.colClient = new(accessmock.AccessAPIClient) suite.execClient = new(accessmock.ExecutionAPIClient) suite.transactionResults = storagemock.NewLightTransactionResults(suite.T()) @@ -910,10 +917,6 @@ func (suite *Suite) TestGetTransactionResultByIndex() { suite.state.On("Final").Return(suite.snapshot, nil).Maybe() suite.snapshot.On("Identities", mock.Anything).Return(fixedENIDs, nil) - // create a mock connection factory - connFactory := connectionmock.NewConnectionFactory(suite.T()) - connFactory.On("GetExecutionAPIClient", mock.Anything).Return(suite.execClient, &mockCloser{}, nil) - exeEventReq := &execproto.GetTransactionByIndexRequest{ BlockId: blockId[:], Index: index, @@ -923,10 +926,11 @@ func (suite *Suite) TestGetTransactionResultByIndex() { Events: nil, } + suite.fixedExecutionNodeIDs = fixedENIDs.NodeIDs() + params := suite.defaultBackendParams() // the connection factory should be used to get the execution node client - params.ConnFactory = connFactory - params.FixedExecutionNodeIDs = (fixedENIDs.NodeIDs()).Strings() + params.ConnFactory = suite.setupConnectionFactory() backend, err := New(params) suite.Require().NoError(err) @@ -964,7 +968,6 @@ func (suite *Suite) TestGetTransactionResultsByBlockID() { suite.state.On("Sealed").Return(suite.snapshot, nil).Maybe() ctx := context.Background() - params := suite.defaultBackendParams() block := unittest.BlockFixture() sporkRootBlockHeight := suite.state.Params().SporkRootBlockHeight() @@ -980,10 +983,6 @@ func (suite *Suite) TestGetTransactionResultsByBlockID() { suite.state.On("Final").Return(suite.snapshot, nil).Maybe() suite.snapshot.On("Identities", mock.Anything).Return(fixedENIDs, nil) - // create a mock connection factory - connFactory := connectionmock.NewConnectionFactory(suite.T()) - connFactory.On("GetExecutionAPIClient", mock.Anything).Return(suite.execClient, &mockCloser{}, nil) - exeEventReq := &execproto.GetTransactionsByBlockIDRequest{ BlockId: blockId[:], } @@ -992,9 +991,11 @@ func (suite *Suite) TestGetTransactionResultsByBlockID() { TransactionResults: []*execproto.GetTransactionResultResponse{{}}, } + suite.fixedExecutionNodeIDs = fixedENIDs.NodeIDs() + + params := suite.defaultBackendParams() // the connection factory should be used to get the execution node client - params.ConnFactory = connFactory - params.FixedExecutionNodeIDs = (fixedENIDs.NodeIDs()).Strings() + params.ConnFactory = suite.setupConnectionFactory() backend, err := New(params) suite.Require().NoError(err) @@ -1073,10 +1074,6 @@ func (suite *Suite) TestTransactionStatusTransition() { suite.state.On("Final").Return(suite.snapshot, nil).Maybe() suite.snapshot.On("Identities", mock.Anything).Return(fixedENIDs, nil) - // create a mock connection factory - connFactory := connectionmock.NewConnectionFactory(suite.T()) - connFactory.On("GetExecutionAPIClient", mock.Anything).Return(suite.execClient, &mockCloser{}, nil) - exeEventReq := &execproto.GetTransactionResultRequest{ BlockId: blockID[:], TransactionId: txID[:], @@ -1086,10 +1083,11 @@ func (suite *Suite) TestTransactionStatusTransition() { Events: nil, } + suite.fixedExecutionNodeIDs = fixedENIDs.NodeIDs() + params := suite.defaultBackendParams() // the connection factory should be used to get the execution node client - params.ConnFactory = connFactory - params.FixedExecutionNodeIDs = (fixedENIDs.NodeIDs()).Strings() + params.ConnFactory = suite.setupConnectionFactory() backend, err := New(params) suite.Require().NoError(err) @@ -1342,12 +1340,11 @@ func (suite *Suite) TestTransactionPendingToFinalizedStatusTransition() { params := suite.defaultBackendParams() params.ConnFactory = connFactory params.MaxHeightRange = TEST_MAX_HEIGHT + suite.preferredExecutionNodeIDs = flow.IdentifierList{receipts[0].ExecutorID} backend, err := New(params) suite.Require().NoError(err) - preferredENIdentifiers = flow.IdentifierList{receipts[0].ExecutorID} - // should return pending status when we have not observed collection for the transaction suite.Run("pending", func() { currentState = flow.TransactionStatusPending @@ -1452,10 +1449,6 @@ func (suite *Suite) TestGetLatestFinalizedBlock() { }) } -type mockCloser struct{} - -func (mc *mockCloser) Close() error { return nil } - func (suite *Suite) TestGetExecutionResultByID() { suite.state.On("Sealed").Return(suite.snapshot, nil).Maybe() @@ -1482,10 +1475,11 @@ func (suite *Suite) TestGetExecutionResultByID() { Return(executionResult, nil) suite.Run("nonexisting execution result for id", func() { + suite.fixedExecutionNodeIDs = validENIDs + params := suite.defaultBackendParams() params.ExecutionResults = results params.ConnFactory = connFactory - params.FixedExecutionNodeIDs = validENIDs.Strings() backend, err := New(params) suite.Require().NoError(err) @@ -1497,10 +1491,11 @@ func (suite *Suite) TestGetExecutionResultByID() { }) suite.Run("existing execution result id", func() { + suite.fixedExecutionNodeIDs = validENIDs + params := suite.defaultBackendParams() params.ExecutionResults = results params.ConnFactory = connFactory - params.FixedExecutionNodeIDs = validENIDs.Strings() backend, err := New(params) suite.Require().NoError(err) @@ -1544,10 +1539,11 @@ func (suite *Suite) TestGetExecutionResultByBlockID() { Return(executionResult, nil) suite.Run("nonexisting execution results", func() { + suite.fixedExecutionNodeIDs = validENIDs + params := suite.defaultBackendParams() params.ExecutionResults = results params.ConnFactory = connFactory - params.FixedExecutionNodeIDs = validENIDs.Strings() backend, err := New(params) suite.Require().NoError(err) @@ -1559,10 +1555,11 @@ func (suite *Suite) TestGetExecutionResultByBlockID() { }) suite.Run("existing execution results", func() { + suite.fixedExecutionNodeIDs = validENIDs + params := suite.defaultBackendParams() params.ExecutionResults = results params.ConnFactory = connFactory - params.FixedExecutionNodeIDs = validENIDs.Strings() backend, err := New(params) suite.Require().NoError(err) @@ -1731,199 +1728,6 @@ func (suite *Suite) TestGetNetworkParameters() { suite.Require().Equal(expectedChainID, actual.ChainID) } -// TestExecutionNodesForBlockID tests the common method backend.executionNodesForBlockID used for serving all API calls -// that need to talk to an execution node. -func (suite *Suite) TestExecutionNodesForBlockID() { - - totalReceipts := 5 - - block := unittest.BlockFixture() - - // generate one execution node identities for each receipt assuming that each ER is generated by a unique exec node - allExecutionNodes := unittest.IdentityListFixture(totalReceipts, unittest.WithRole(flow.RoleExecution)) - - // one execution result for all receipts for this block - executionResult := unittest.ExecutionResultFixture() - - // generate execution receipts - receipts := make(flow.ExecutionReceiptList, totalReceipts) - for j := 0; j < totalReceipts; j++ { - r := unittest.ReceiptForBlockFixture(&block) - r.ExecutorID = allExecutionNodes[j].NodeID - er := *executionResult - r.ExecutionResult = er - receipts[j] = r - } - - currentAttempt := 0 - attempt1Receipts, attempt2Receipts, attempt3Receipts := receipts, receipts, receipts - - // setup receipts storage mock to return different list of receipts on each call - suite.receipts. - On("ByBlockID", block.ID()).Return( - func(id flow.Identifier) flow.ExecutionReceiptList { - switch currentAttempt { - case 0: - currentAttempt++ - return attempt1Receipts - case 1: - currentAttempt++ - return attempt2Receipts - default: - currentAttempt = 0 - return attempt3Receipts - } - }, - func(id flow.Identifier) error { return nil }) - - suite.snapshot.On("Identities", mock.Anything).Return( - func(filter flow.IdentityFilter[flow.Identity]) flow.IdentityList { - // apply the filter passed in to the list of all the execution nodes - return allExecutionNodes.Filter(filter) - }, - func(flow.IdentityFilter[flow.Identity]) error { return nil }) - suite.state.On("Final").Return(suite.snapshot, nil).Maybe() - - testExecutionNodesForBlockID := func(preferredENs, fixedENs, expectedENs flow.IdentityList) { - - if preferredENs != nil { - preferredENIdentifiers = preferredENs.NodeIDs() - } - if fixedENs != nil { - fixedENIdentifiers = fixedENs.NodeIDs() - } - - if expectedENs == nil { - expectedENs = flow.IdentityList{} - } - - allExecNodes, err := executionNodesForBlockID(context.Background(), block.ID(), suite.receipts, suite.state, suite.log) - require.NoError(suite.T(), err) - - execNodeSelectorFactory := NodeSelectorFactory{circuitBreakerEnabled: false} - execSelector, err := execNodeSelectorFactory.SelectNodes(allExecNodes) - require.NoError(suite.T(), err) - - actualList := flow.IdentitySkeletonList{} - for actual := execSelector.Next(); actual != nil; actual = execSelector.Next() { - actualList = append(actualList, actual) - } - - { - expectedENs := expectedENs.ToSkeleton() - if len(expectedENs) > maxNodesCnt { - for _, actual := range actualList { - require.Contains(suite.T(), expectedENs, actual) - } - } else { - require.ElementsMatch(suite.T(), actualList, expectedENs) - } - } - } - // if we don't find sufficient receipts, executionNodesForBlockID should return a list of random ENs - suite.Run("insufficient receipts return random ENs in State", func() { - // return no receipts at all attempts - attempt1Receipts = flow.ExecutionReceiptList{} - attempt2Receipts = flow.ExecutionReceiptList{} - attempt3Receipts = flow.ExecutionReceiptList{} - suite.state.On("AtBlockID", mock.Anything).Return(suite.snapshot) - - allExecNodes, err := executionNodesForBlockID(context.Background(), block.ID(), suite.receipts, suite.state, suite.log) - require.NoError(suite.T(), err) - - execNodeSelectorFactory := NodeSelectorFactory{circuitBreakerEnabled: false} - execSelector, err := execNodeSelectorFactory.SelectNodes(allExecNodes) - require.NoError(suite.T(), err) - - actualList := flow.IdentitySkeletonList{} - for actual := execSelector.Next(); actual != nil; actual = execSelector.Next() { - actualList = append(actualList, actual) - } - - require.Equal(suite.T(), len(actualList), maxNodesCnt) - }) - - // if no preferred or fixed ENs are specified, the ExecutionNodesForBlockID function should - // return the exe node list without a filter - suite.Run("no preferred or fixed ENs", func() { - testExecutionNodesForBlockID(nil, nil, allExecutionNodes) - }) - // if only fixed ENs are specified, the ExecutionNodesForBlockID function should - // return the fixed ENs list - suite.Run("two fixed ENs with zero preferred EN", func() { - // mark the first two ENs as fixed - fixedENs := allExecutionNodes[0:2] - expectedList := fixedENs - testExecutionNodesForBlockID(nil, fixedENs, expectedList) - }) - // if only preferred ENs are specified, the ExecutionNodesForBlockID function should - // return the preferred ENs list - suite.Run("two preferred ENs with zero fixed EN", func() { - // mark the first two ENs as preferred - preferredENs := allExecutionNodes[0:2] - expectedList := allExecutionNodes[0:maxNodesCnt] - testExecutionNodesForBlockID(preferredENs, nil, expectedList) - }) - // if both are specified, the ExecutionNodesForBlockID function should - // return the preferred ENs list - suite.Run("four fixed ENs of which two are preferred ENs", func() { - // mark the first four ENs as fixed - fixedENs := allExecutionNodes[0:5] - // mark the first two of the fixed ENs as preferred ENs - preferredENs := fixedENs[0:2] - expectedList := fixedENs[0:maxNodesCnt] - testExecutionNodesForBlockID(preferredENs, fixedENs, expectedList) - }) - // if both are specified, but the preferred ENs don't match the ExecutorIDs in the ER, - // the ExecutionNodesForBlockID function should return the fixed ENs list - suite.Run("four fixed ENs of which two are preferred ENs but have not generated the ER", func() { - // mark the first two ENs as fixed - fixedENs := allExecutionNodes[0:2] - // specify two ENs not specified in the ERs as preferred - preferredENs := unittest.IdentityListFixture(2, unittest.WithRole(flow.RoleExecution)) - // add one more node ID besides of the fixed ENs list cause expected length of the list should be maxNodesCnt - expectedList := append(fixedENs, allExecutionNodes[2]) - testExecutionNodesForBlockID(preferredENs, fixedENs, expectedList) - }) - // if execution receipts are not yet available, the ExecutionNodesForBlockID function should retry twice - suite.Run("retry execution receipt query", func() { - // on first attempt, no execution receipts are available - attempt1Receipts = flow.ExecutionReceiptList{} - // on second attempt ony one is available - attempt2Receipts = flow.ExecutionReceiptList{receipts[0]} - // on third attempt all receipts are available - attempt3Receipts = receipts - currentAttempt = 0 - // mark the first two ENs as preferred - preferredENs := allExecutionNodes[0:2] - expectedList := allExecutionNodes[0:maxNodesCnt] - testExecutionNodesForBlockID(preferredENs, nil, expectedList) - }) - // if preferredENIdentifiers was set and there are less than maxNodesCnt nodes selected than check the order - // of adding ENs ids - suite.Run("add nodes in the correct order", func() { - // mark the first EN as preferred - preferredENIdentifiers = allExecutionNodes[0:1].NodeIDs() - // mark the fourth EN with receipt - executorIDs := allExecutionNodes[3:4].NodeIDs() - - receiptNodes := allExecutionNodes[3:4] // any EN with a receipt - preferredNodes := allExecutionNodes[0:1] // preferred EN node not already selected - additionalNode := allExecutionNodes[1:2] // any EN not already selected - - expectedOrder := flow.IdentityList{ - receiptNodes[0], - preferredNodes[0], - additionalNode[0], - } - - chosenIDs := chooseFromPreferredENIDs(allExecutionNodes, executorIDs) - - require.ElementsMatch(suite.T(), chosenIDs, expectedOrder) - require.Equal(suite.T(), len(chosenIDs), maxNodesCnt) - }) -} - // TestGetTransactionResultEventEncodingVersion tests the GetTransactionResult function with different event encoding versions. func (suite *Suite) TestGetTransactionResultEventEncodingVersion() { suite.state.On("Sealed").Return(suite.snapshot, nil).Maybe() @@ -1966,14 +1770,11 @@ func (suite *Suite) TestGetTransactionResultEventEncodingVersion() { suite.state.On("Final").Return(suite.snapshot, nil).Maybe() suite.snapshot.On("Identities", mock.Anything).Return(fixedENIDs, nil) - // create a mock connection factory - connFactory := connectionmock.NewConnectionFactory(suite.T()) - connFactory.On("GetExecutionAPIClient", mock.Anything).Return(suite.execClient, &mockCloser{}, nil) + suite.fixedExecutionNodeIDs = fixedENIDs.NodeIDs() params := suite.defaultBackendParams() // the connection factory should be used to get the execution node client - params.ConnFactory = connFactory - params.FixedExecutionNodeIDs = (fixedENIDs.NodeIDs()).Strings() + params.ConnFactory = suite.setupConnectionFactory() backend, err := New(params) suite.Require().NoError(err) @@ -2032,14 +1833,11 @@ func (suite *Suite) TestGetTransactionResultByIndexAndBlockIdEventEncodingVersio suite.state.On("Final").Return(suite.snapshot, nil).Maybe() suite.snapshot.On("Identities", mock.Anything).Return(fixedENIDs, nil) - // create a mock connection factory - connFactory := connectionmock.NewConnectionFactory(suite.T()) - connFactory.On("GetExecutionAPIClient", mock.Anything).Return(suite.execClient, &mockCloser{}, nil) + suite.fixedExecutionNodeIDs = fixedENIDs.NodeIDs() params := suite.defaultBackendParams() // the connection factory should be used to get the execution node client - params.ConnFactory = connFactory - params.FixedExecutionNodeIDs = (fixedENIDs.NodeIDs()).Strings() + params.ConnFactory = suite.setupConnectionFactory() backend, err := New(params) suite.Require().NoError(err) @@ -2132,20 +1930,17 @@ func (suite *Suite) TestNodeCommunicator() { suite.state.On("Final").Return(suite.snapshot, nil).Maybe() suite.snapshot.On("Identities", mock.Anything).Return(fixedENIDs, nil) - // create a mock connection factory - connFactory := connectionmock.NewConnectionFactory(suite.T()) - connFactory.On("GetExecutionAPIClient", mock.Anything).Return(suite.execClient, &mockCloser{}, nil) - exeEventReq := &execproto.GetTransactionsByBlockIDRequest{ BlockId: blockId[:], } + // Left only one preferred execution node + suite.fixedExecutionNodeIDs = fixedENIDs.NodeIDs() + suite.preferredExecutionNodeIDs = flow.IdentifierList{fixedENIDs[0].NodeID} + params := suite.defaultBackendParams() // the connection factory should be used to get the execution node client - params.ConnFactory = connFactory - params.FixedExecutionNodeIDs = (fixedENIDs.NodeIDs()).Strings() - // Left only one preferred execution node - params.PreferredExecutionNodeIDs = []string{fixedENIDs[0].NodeID.String()} + params.ConnFactory = suite.setupConnectionFactory() backend, err := New(params) suite.Require().NoError(err) @@ -2195,7 +1990,7 @@ func (suite *Suite) setupReceipts(block *flow.Block) ([]*flow.ExecutionReceipt, func (suite *Suite) setupConnectionFactory() connection.ConnectionFactory { // create a mock connection factory connFactory := connectionmock.NewConnectionFactory(suite.T()) - connFactory.On("GetExecutionAPIClient", mock.Anything).Return(suite.execClient, &mockCloser{}, nil) + connFactory.On("GetExecutionAPIClient", mock.Anything).Return(suite.execClient, &mocks.MockCloser{}, nil) return connFactory } @@ -2220,24 +2015,30 @@ func generateEncodedEvents(t *testing.T, n int) ([]flow.Event, []flow.Event) { func (suite *Suite) defaultBackendParams() Params { return Params{ - State: suite.state, - Blocks: suite.blocks, - Headers: suite.headers, - Collections: suite.collections, - Transactions: suite.transactions, - ExecutionReceipts: suite.receipts, - ExecutionResults: suite.results, - ChainID: suite.chainID, - CollectionRPC: suite.colClient, - MaxHeightRange: DefaultMaxHeightRange, - SnapshotHistoryLimit: DefaultSnapshotHistoryLimit, - Communicator: NewNodeCommunicator(false), - AccessMetrics: metrics.NewNoopCollector(), - Log: suite.log, - TxErrorMessagesCacheSize: 1000, - BlockTracker: nil, - TxResultQueryMode: IndexQueryModeExecutionNodesOnly, - LastFullBlockHeight: suite.lastFullBlockHeight, - VersionControl: suite.versionControl, + State: suite.state, + Blocks: suite.blocks, + Headers: suite.headers, + Collections: suite.collections, + Transactions: suite.transactions, + ExecutionReceipts: suite.receipts, + ExecutionResults: suite.results, + ChainID: suite.chainID, + CollectionRPC: suite.colClient, + MaxHeightRange: DefaultMaxHeightRange, + SnapshotHistoryLimit: DefaultSnapshotHistoryLimit, + Communicator: NewNodeCommunicator(false), + AccessMetrics: metrics.NewNoopCollector(), + Log: suite.log, + BlockTracker: nil, + TxResultQueryMode: IndexQueryModeExecutionNodesOnly, + LastFullBlockHeight: suite.lastFullBlockHeight, + VersionControl: suite.versionControl, + ExecNodeIdentitiesProvider: commonrpc.NewExecutionNodeIdentitiesProvider( + suite.log, + suite.state, + suite.receipts, + suite.preferredExecutionNodeIDs, + suite.fixedExecutionNodeIDs, + ), } } diff --git a/engine/access/rpc/backend/backend_transactions.go b/engine/access/rpc/backend/backend_transactions.go index 7436863f9af..02a6b312e8e 100644 --- a/engine/access/rpc/backend/backend_transactions.go +++ b/engine/access/rpc/backend/backend_transactions.go @@ -17,6 +17,7 @@ import ( "github.com/onflow/flow-go/access" "github.com/onflow/flow-go/engine/access/rpc/connection" "github.com/onflow/flow-go/engine/common/rpc" + commonrpc "github.com/onflow/flow-go/engine/common/rpc" "github.com/onflow/flow-go/engine/common/rpc/convert" "github.com/onflow/flow-go/model/flow" "github.com/onflow/flow-go/module" @@ -25,26 +26,31 @@ import ( "github.com/onflow/flow-go/storage" ) +const FailedErrorMessage = "failed" + type backendTransactions struct { *TransactionsLocalDataProvider - staticCollectionRPC accessproto.AccessAPIClient // rpc client tied to a fixed collection node - transactions storage.Transactions - executionReceipts storage.ExecutionReceipts - chainID flow.ChainID - transactionMetrics module.TransactionMetrics - transactionValidator *access.TransactionValidator - retry *Retry - connFactory connection.ConnectionFactory - - previousAccessNodes []accessproto.AccessAPIClient - log zerolog.Logger - nodeCommunicator Communicator - txResultCache *lru.Cache[flow.Identifier, *access.TransactionResult] - txErrorMessagesCache *lru.Cache[flow.Identifier, string] // cache for transactions error messages, indexed by hash(block_id, tx_id). - txResultQueryMode IndexQueryMode - - systemTxID flow.Identifier - systemTx *flow.TransactionBody + staticCollectionRPC accessproto.AccessAPIClient // rpc client tied to a fixed collection node + transactions storage.Transactions + // NOTE: The transaction error message is currently only used by the access node and not by the observer node. + // To avoid introducing unnecessary command line arguments in the observer, one case could be that the error + // message cache is nil for the observer node. + txResultErrorMessages storage.TransactionResultErrorMessages + chainID flow.ChainID + transactionMetrics module.TransactionMetrics + transactionValidator *access.TransactionValidator + retry *Retry + connFactory connection.ConnectionFactory + + previousAccessNodes []accessproto.AccessAPIClient + log zerolog.Logger + nodeCommunicator Communicator + txResultCache *lru.Cache[flow.Identifier, *access.TransactionResult] + txResultQueryMode IndexQueryMode + + systemTxID flow.Identifier + systemTx *flow.TransactionBody + execNodeIdentitiesProvider *commonrpc.ExecutionNodeIdentitiesProvider } var _ TransactionErrorMessage = (*backendTransactions)(nil) @@ -405,7 +411,10 @@ func (b *backendTransactions) getTransactionResultsByBlockIDFromExecutionNode( BlockId: blockID[:], } - execNodes, err := executionNodesForBlockID(ctx, blockID, b.executionReceipts, b.state, b.log) + execNodes, err := b.execNodeIdentitiesProvider.ExecutionNodesForBlockID( + ctx, + blockID, + ) if err != nil { if IsInsufficientExecutionReceipts(err) { return nil, status.Errorf(codes.NotFound, err.Error()) @@ -559,7 +568,10 @@ func (b *backendTransactions) getTransactionResultByIndexFromExecutionNode( Index: index, } - execNodes, err := executionNodesForBlockID(ctx, blockID, b.executionReceipts, b.state, b.log) + execNodes, err := b.execNodeIdentitiesProvider.ExecutionNodesForBlockID( + ctx, + blockID, + ) if err != nil { if IsInsufficientExecutionReceipts(err) { return nil, status.Errorf(codes.NotFound, err.Error()) @@ -743,7 +755,10 @@ func (b *backendTransactions) getTransactionResultFromExecutionNode( TransactionId: transactionID[:], } - execNodes, err := executionNodesForBlockID(ctx, blockID, b.executionReceipts, b.state, b.log) + execNodes, err := b.execNodeIdentitiesProvider.ExecutionNodesForBlockID( + ctx, + blockID, + ) if err != nil { // if no execution receipt were found, return a NotFound GRPC error if IsInsufficientExecutionReceipts(err) { @@ -970,20 +985,20 @@ func (b *backendTransactions) tryGetTransactionResultByIndex( func (b *backendTransactions) LookupErrorMessageByTransactionID( ctx context.Context, blockID flow.Identifier, + height uint64, transactionID flow.Identifier, ) (string, error) { - var cacheKey flow.Identifier - var value string - - if b.txErrorMessagesCache != nil { - cacheKey = flow.MakeIDFromFingerPrint(append(blockID[:], transactionID[:]...)) - value, cached := b.txErrorMessagesCache.Get(cacheKey) - if cached { - return value, nil + if b.txResultErrorMessages != nil { + res, err := b.txResultErrorMessages.ByBlockIDTransactionID(blockID, transactionID) + if err == nil { + return res.ErrorMessage, nil } } - execNodes, err := executionNodesForBlockID(ctx, blockID, b.executionReceipts, b.state, b.log) + execNodes, err := b.execNodeIdentitiesProvider.ExecutionNodesForBlockID( + ctx, + blockID, + ) if err != nil { if IsInsufficientExecutionReceipts(err) { return "", status.Errorf(codes.NotFound, err.Error()) @@ -997,22 +1012,28 @@ func (b *backendTransactions) LookupErrorMessageByTransactionID( resp, err := b.getTransactionErrorMessageFromAnyEN(ctx, execNodes, req) if err != nil { - return "", fmt.Errorf("could not fetch error message from ENs: %w", err) - } - value = resp.ErrorMessage + // If no execution nodes return a valid response, + // return a static message "failed". + txResult, err := b.txResultsIndex.ByBlockIDTransactionID(blockID, height, transactionID) + if err != nil { + return "", rpc.ConvertStorageError(err) + } - if b.txErrorMessagesCache != nil { - b.txErrorMessagesCache.Add(cacheKey, value) + if txResult.Failed { + return FailedErrorMessage, nil + } + + // in case tx result is not failed + return "", nil } - return value, nil + return resp.ErrorMessage, nil } // LookupErrorMessageByIndex returns transaction error message for specified transaction using its index. // If an error message for transaction can be found in cache then it will be used to serve the request, otherwise // an RPC call will be made to the EN to fetch that error message, fetched value will be cached in the LRU cache. // Expected errors during normal operation: -// - status.Error[codes.NotFound] - transaction result for given block ID and tx index is not available. // - InsufficientExecutionReceipts - found insufficient receipts for given block ID. // - status.Error - remote GRPC call to EN has failed. func (b *backendTransactions) LookupErrorMessageByIndex( @@ -1021,23 +1042,17 @@ func (b *backendTransactions) LookupErrorMessageByIndex( height uint64, index uint32, ) (string, error) { - txResult, err := b.txResultsIndex.ByBlockIDTransactionIndex(blockID, height, index) - if err != nil { - return "", rpc.ConvertStorageError(err) - } - - var cacheKey flow.Identifier - var value string - - if b.txErrorMessagesCache != nil { - cacheKey = flow.MakeIDFromFingerPrint(append(blockID[:], txResult.TransactionID[:]...)) - value, cached := b.txErrorMessagesCache.Get(cacheKey) - if cached { - return value, nil + if b.txResultErrorMessages != nil { + res, err := b.txResultErrorMessages.ByBlockIDTransactionIndex(blockID, index) + if err == nil { + return res.ErrorMessage, nil } } - execNodes, err := executionNodesForBlockID(ctx, blockID, b.executionReceipts, b.state, b.log) + execNodes, err := b.execNodeIdentitiesProvider.ExecutionNodesForBlockID( + ctx, + blockID, + ) if err != nil { if IsInsufficientExecutionReceipts(err) { return "", status.Errorf(codes.NotFound, err.Error()) @@ -1051,21 +1066,27 @@ func (b *backendTransactions) LookupErrorMessageByIndex( resp, err := b.getTransactionErrorMessageByIndexFromAnyEN(ctx, execNodes, req) if err != nil { - return "", fmt.Errorf("could not fetch error message from ENs: %w", err) - } - value = resp.ErrorMessage + // If no execution nodes return a valid response, + // return a static message "failed" + txResult, err := b.txResultsIndex.ByBlockIDTransactionIndex(blockID, height, index) + if err != nil { + return "", rpc.ConvertStorageError(err) + } + + if txResult.Failed { + return FailedErrorMessage, nil + } - if b.txErrorMessagesCache != nil { - b.txErrorMessagesCache.Add(cacheKey, value) + // in case tx result is not failed + return "", nil } - return value, nil + return resp.ErrorMessage, nil } // LookupErrorMessagesByBlockID returns all error messages for failed transactions by blockID. // An RPC call will be made to the EN to fetch missing errors messages, fetched value will be cached in the LRU cache. // Expected errors during normal operation: -// - status.Error[codes.NotFound] - transaction results for given block ID are not available. // - InsufficientExecutionReceipts - found insufficient receipts for given block ID. // - status.Error - remote GRPC call to EN has failed. func (b *backendTransactions) LookupErrorMessagesByBlockID( @@ -1073,33 +1094,23 @@ func (b *backendTransactions) LookupErrorMessagesByBlockID( blockID flow.Identifier, height uint64, ) (map[flow.Identifier]string, error) { - txResults, err := b.txResultsIndex.ByBlockID(blockID, height) - if err != nil { - return nil, rpc.ConvertStorageError(err) - } - - results := make(map[flow.Identifier]string) + result := make(map[flow.Identifier]string) - if b.txErrorMessagesCache != nil { - needToFetch := false - for _, txResult := range txResults { - if txResult.Failed { - cacheKey := flow.MakeIDFromFingerPrint(append(blockID[:], txResult.TransactionID[:]...)) - if value, ok := b.txErrorMessagesCache.Get(cacheKey); ok { - results[txResult.TransactionID] = value - } else { - needToFetch = true - } + if b.txResultErrorMessages != nil { + res, err := b.txResultErrorMessages.ByBlockID(blockID) + if err == nil { + for _, value := range res { + result[value.TransactionID] = value.ErrorMessage } - } - // all transactions were served from cache or there were no failed transactions - if !needToFetch { - return results, nil + return result, nil } } - execNodes, err := executionNodesForBlockID(ctx, blockID, b.executionReceipts, b.state, b.log) + execNodes, err := b.execNodeIdentitiesProvider.ExecutionNodesForBlockID( + ctx, + blockID, + ) if err != nil { if IsInsufficientExecutionReceipts(err) { return nil, status.Errorf(codes.NotFound, err.Error()) @@ -1110,18 +1121,28 @@ func (b *backendTransactions) LookupErrorMessagesByBlockID( BlockId: convert.IdentifierToMessage(blockID), } - resp, err := b.getTransactionErrorMessagesFromAnyEN(ctx, execNodes, req) + resp, _, err := b.GetTransactionErrorMessagesFromAnyEN(ctx, execNodes, req) if err != nil { - return nil, fmt.Errorf("could not fetch error message from ENs: %w", err) + // If no execution nodes return a valid response, + // return a static message "failed" + txResults, err := b.txResultsIndex.ByBlockID(blockID, height) + if err != nil { + return nil, rpc.ConvertStorageError(err) + } + + for _, txResult := range txResults { + if txResult.Failed { + result[txResult.TransactionID] = FailedErrorMessage + } + } + + return result, nil } - result := make(map[flow.Identifier]string, len(resp)) + for _, value := range resp { - if b.txErrorMessagesCache != nil { - cacheKey := flow.MakeIDFromFingerPrint(append(req.BlockId, value.TransactionId...)) - b.txErrorMessagesCache.Add(cacheKey, value.ErrorMessage) - } result[convert.MessageToIdentifier(value.TransactionId)] = value.ErrorMessage } + return result, nil } @@ -1209,26 +1230,29 @@ func (b *backendTransactions) getTransactionErrorMessageByIndexFromAnyEN( return resp, nil } -// getTransactionErrorMessagesFromAnyEN performs an RPC call using available nodes passed as argument. List of nodes must be non-empty otherwise an error will be returned. +// GetTransactionErrorMessagesFromAnyEN performs an RPC call using available nodes passed as argument. List of nodes must be non-empty otherwise an error will be returned. // Expected errors during normal operation: // - status.Error - GRPC call failed, some of possible codes are: // - codes.NotFound - request cannot be served by EN because of absence of data. // - codes.Unavailable - remote node is not unavailable. -func (b *backendTransactions) getTransactionErrorMessagesFromAnyEN( +func (b *backendTransactions) GetTransactionErrorMessagesFromAnyEN( ctx context.Context, execNodes flow.IdentitySkeletonList, req *execproto.GetTransactionErrorMessagesByBlockIDRequest, -) ([]*execproto.GetTransactionErrorMessagesResponse_Result, error) { +) ([]*execproto.GetTransactionErrorMessagesResponse_Result, *flow.IdentitySkeleton, error) { // if we were passed 0 execution nodes add a specific error if len(execNodes) == 0 { - return nil, errors.New("zero execution nodes") + return nil, nil, errors.New("zero execution nodes") } var resp *execproto.GetTransactionErrorMessagesResponse + var execNode *flow.IdentitySkeleton + errToReturn := b.nodeCommunicator.CallAvailableNode( execNodes, func(node *flow.IdentitySkeleton) error { var err error + execNode = node resp, err = b.tryGetTransactionErrorMessagesByBlockIDFromEN(ctx, node, req) if err == nil { b.log.Debug(). @@ -1245,10 +1269,10 @@ func (b *backendTransactions) getTransactionErrorMessagesFromAnyEN( // log the errors if errToReturn != nil { b.log.Err(errToReturn).Msg("failed to get transaction error messages from execution nodes") - return nil, errToReturn + return nil, nil, errToReturn } - return resp.GetResults(), nil + return resp.GetResults(), execNode, nil } // Expected errors during normal operation: diff --git a/engine/access/rpc/backend/backend_transactions_test.go b/engine/access/rpc/backend/backend_transactions_test.go index 5d1c513cef8..6eb631d093a 100644 --- a/engine/access/rpc/backend/backend_transactions_test.go +++ b/engine/access/rpc/backend/backend_transactions_test.go @@ -29,6 +29,7 @@ import ( "github.com/onflow/flow-go/storage" "github.com/onflow/flow-go/utils/unittest" "github.com/onflow/flow-go/utils/unittest/generator" + "github.com/onflow/flow-go/utils/unittest/mocks" ) const expectedErrorMsg = "expected test error" @@ -338,96 +339,193 @@ func (suite *Suite) TestGetTransactionResultUnknownFromCache() { }) } -// TestLookupTransactionErrorMessage_HappyPath tests lookup of a transaction error message. In a happy path if it wasn't found in the cache, it -// has to be fetched from the execution node, otherwise served from the cache. -// If the transaction has not failed, the error message must be empty. -func (suite *Suite) TestLookupTransactionErrorMessage_HappyPath() { +// TestLookupTransactionErrorMessageByTransactionID_HappyPath verifies the lookup of a transaction error message +// by block id and transaction id. +// It tests two cases: +// 1. Happy path where the error message is fetched from the EN if it's not found in the cache. +// 2. Happy path where the error message is served from the storage database if it exists. +func (suite *Suite) TestLookupTransactionErrorMessageByTransactionID_HappyPath() { block := unittest.BlockFixture() blockId := block.ID() failedTx := unittest.TransactionFixture() failedTxId := failedTx.ID() + failedTxIndex := rand.Uint32() + // Setup mock receipts and execution node identities. _, fixedENIDs := suite.setupReceipts(&block) suite.state.On("Final").Return(suite.snapshot, nil).Maybe() suite.snapshot.On("Identities", mock.Anything).Return(fixedENIDs, nil) - // create a mock connection factory - connFactory := connectionmock.NewConnectionFactory(suite.T()) - connFactory.On("GetExecutionAPIClient", mock.Anything).Return(suite.execClient, &mockCloser{}, nil) + suite.fixedExecutionNodeIDs = fixedENIDs.NodeIDs() params := suite.defaultBackendParams() - // the connection factory should be used to get the execution node client - params.ConnFactory = connFactory - params.FixedExecutionNodeIDs = fixedENIDs.NodeIDs().Strings() + params.TxResultErrorMessages = suite.txErrorMessages - backend, err := New(params) - suite.Require().NoError(err) + // Test case: transaction error message is fetched from the EN. + suite.Run("happy path from EN", func() { + // the connection factory should be used to get the execution node client + params.ConnFactory = suite.setupConnectionFactory() - expectedErrorMsg := "some error" + // Mock the cache lookup for the transaction error message, returning "not found". + suite.txErrorMessages.On("ByBlockIDTransactionID", blockId, failedTxId). + Return(nil, storage.ErrNotFound).Once() - exeEventReq := &execproto.GetTransactionErrorMessageRequest{ - BlockId: blockId[:], - TransactionId: failedTxId[:], - } + backend, err := New(params) + suite.Require().NoError(err) - exeEventResp := &execproto.GetTransactionErrorMessageResponse{ - TransactionId: failedTxId[:], - ErrorMessage: expectedErrorMsg, - } + // Mock the execution node API call to fetch the error message. + exeEventReq := &execproto.GetTransactionErrorMessageRequest{ + BlockId: blockId[:], + TransactionId: failedTxId[:], + } + exeEventResp := &execproto.GetTransactionErrorMessageResponse{ + TransactionId: failedTxId[:], + ErrorMessage: expectedErrorMsg, + } + suite.execClient.On("GetTransactionErrorMessage", mock.Anything, exeEventReq).Return(exeEventResp, nil).Once() - suite.execClient.On("GetTransactionErrorMessage", mock.Anything, exeEventReq).Return(exeEventResp, nil).Once() + // Perform the lookup and assert that the error message is retrieved correctly. + errMsg, err := backend.LookupErrorMessageByTransactionID(context.Background(), blockId, block.Header.Height, failedTxId) + suite.Require().NoError(err) + suite.Require().Equal(expectedErrorMsg, errMsg) + suite.assertAllExpectations() + }) - errMsg, err := backend.LookupErrorMessageByTransactionID(context.Background(), blockId, failedTxId) - suite.Require().NoError(err) - suite.Require().Equal(expectedErrorMsg, errMsg) + // Test case: transaction error message is fetched from the storage database. + suite.Run("happy path from storage db", func() { + backend, err := New(params) + suite.Require().NoError(err) - // ensure the transaction error message is cached after retrieval; we do this by mocking the grpc call - // only once - errMsg, err = backend.LookupErrorMessageByTransactionID(context.Background(), blockId, failedTxId) - suite.Require().NoError(err) - suite.Require().Equal(expectedErrorMsg, errMsg) - suite.assertAllExpectations() + // Mock the cache lookup for the transaction error message, returning a stored result. + suite.txErrorMessages.On("ByBlockIDTransactionID", blockId, failedTxId). + Return(&flow.TransactionResultErrorMessage{ + TransactionID: failedTxId, + ErrorMessage: expectedErrorMsg, + Index: failedTxIndex, + ExecutorID: unittest.IdentifierFixture(), + }, nil).Once() + + // Perform the lookup and assert that the error message is retrieved correctly from storage. + errMsg, err := backend.LookupErrorMessageByTransactionID(context.Background(), blockId, block.Header.Height, failedTxId) + suite.Require().NoError(err) + suite.Require().Equal(expectedErrorMsg, errMsg) + suite.assertAllExpectations() + }) } -// TestLookupTransactionErrorMessage_FailedToFetch tests lookup of a transaction error message, when a transaction result -// is not in the cache and needs to be fetched from EN, but the EN fails to return it. -func (suite *Suite) TestLookupTransactionErrorMessage_FailedToFetch() { +// TestLookupTransactionErrorMessageByTransactionID_FailedToFetch tests the case when a transaction error message +// is not in the cache and needs to be fetched from the EN, but the EN fails to return it. +// It tests three cases: +// 1. The transaction is not found in the transaction results, leading to a "NotFound" error. +// 2. The transaction result is not failed, and the error message is empty. +// 3. The transaction result is failed, and the error message "failed" are returned. +func (suite *Suite) TestLookupTransactionErrorMessageByTransactionID_FailedToFetch() { block := unittest.BlockFixture() blockId := block.ID() failedTx := unittest.TransactionFixture() failedTxId := failedTx.ID() + // Setup mock receipts and execution node identities. _, fixedENIDs := suite.setupReceipts(&block) suite.state.On("Final").Return(suite.snapshot, nil).Maybe() suite.snapshot.On("Identities", mock.Anything).Return(fixedENIDs, nil) - // create a mock connection factory - connFactory := connectionmock.NewConnectionFactory(suite.T()) - connFactory.On("GetExecutionAPIClient", mock.Anything).Return(suite.execClient, &mockCloser{}, nil) + // Create a mock index reporter + reporter := syncmock.NewIndexReporter(suite.T()) + reporter.On("LowestIndexedHeight").Return(block.Header.Height, nil) + reporter.On("HighestIndexedHeight").Return(block.Header.Height+10, nil) + + suite.fixedExecutionNodeIDs = fixedENIDs.NodeIDs() params := suite.defaultBackendParams() - // the connection factory should be used to get the execution node client - params.ConnFactory = connFactory - params.FixedExecutionNodeIDs = fixedENIDs.NodeIDs().Strings() + // The connection factory should be used to get the execution node client + params.ConnFactory = suite.setupConnectionFactory() + // Initialize the transaction results index with the mock reporter. + params.TxResultsIndex = index.NewTransactionResultsIndex(index.NewReporter(), suite.transactionResults) + err := params.TxResultsIndex.Initialize(reporter) + suite.Require().NoError(err) + + params.TxResultErrorMessages = suite.txErrorMessages backend, err := New(params) suite.Require().NoError(err) - // lookup should try each of the 2 ENs in fixedENIDs - suite.execClient.On("GetTransactionErrorMessage", mock.Anything, mock.Anything).Return(nil, - status.Error(codes.Unavailable, "")).Twice() + // Test case: failed to fetch from EN, transaction is unknown. + suite.Run("failed to fetch from EN, unknown tx", func() { + // lookup should try each of the 2 ENs in fixedENIDs + suite.execClient.On("GetTransactionErrorMessage", mock.Anything, mock.Anything).Return(nil, + status.Error(codes.Unavailable, "")).Twice() - errMsg, err := backend.LookupErrorMessageByTransactionID(context.Background(), blockId, failedTxId) - suite.Require().Error(err) - suite.Require().Equal(codes.Unavailable, status.Code(err)) - suite.Require().Empty(errMsg) + // Setup mock that the transaction and tx error message is not found in the storage. + suite.txErrorMessages.On("ByBlockIDTransactionID", blockId, failedTxId). + Return(nil, storage.ErrNotFound).Once() + suite.transactionResults.On("ByBlockIDTransactionID", blockId, failedTxId). + Return(nil, storage.ErrNotFound).Once() + + // Perform the lookup and expect a "NotFound" error with an empty error message. + errMsg, err := backend.LookupErrorMessageByTransactionID(context.Background(), blockId, block.Header.Height, failedTxId) + suite.Require().Error(err) + suite.Require().Equal(codes.NotFound, status.Code(err)) + suite.Require().Empty(errMsg) + suite.assertAllExpectations() + }) + + // Test case: failed to fetch from EN, but the transaction result is not failed. + suite.Run("failed to fetch from EN, tx result is not failed", func() { + // Lookup should try each of the 2 ENs in fixedENIDs + suite.execClient.On("GetTransactionErrorMessage", mock.Anything, mock.Anything).Return(nil, + status.Error(codes.Unavailable, "")).Twice() + + // Setup mock that the transaction error message is not found in storage. + suite.txErrorMessages.On("ByBlockIDTransactionID", blockId, failedTxId). + Return(nil, storage.ErrNotFound).Once() + + // Setup mock that the transaction result exists and is not failed. + suite.transactionResults.On("ByBlockIDTransactionID", blockId, failedTxId). + Return(&flow.LightTransactionResult{ + TransactionID: failedTxId, + Failed: false, + ComputationUsed: 0, + }, nil).Once() + + // Perform the lookup and expect no error and an empty error message. + errMsg, err := backend.LookupErrorMessageByTransactionID(context.Background(), blockId, block.Header.Height, failedTxId) + suite.Require().NoError(err) + suite.Require().Empty(errMsg) + suite.assertAllExpectations() + }) - suite.assertAllExpectations() + // Test case: failed to fetch from EN, but the transaction result is failed. + suite.Run("failed to fetch from EN, tx result is failed", func() { + // lookup should try each of the 2 ENs in fixedENIDs + suite.execClient.On("GetTransactionErrorMessage", mock.Anything, mock.Anything).Return(nil, + status.Error(codes.Unavailable, "")).Twice() + + // Setup mock that the transaction error message is not found in storage. + suite.txErrorMessages.On("ByBlockIDTransactionID", blockId, failedTxId). + Return(nil, storage.ErrNotFound).Once() + + // Setup mock that the transaction result exists and is failed. + suite.transactionResults.On("ByBlockIDTransactionID", blockId, failedTxId). + Return(&flow.LightTransactionResult{ + TransactionID: failedTxId, + Failed: true, + ComputationUsed: 0, + }, nil).Once() + + // Perform the lookup and expect the failed error message to be returned. + errMsg, err := backend.LookupErrorMessageByTransactionID(context.Background(), blockId, block.Header.Height, failedTxId) + suite.Require().NoError(err) + suite.Require().Equal(errMsg, FailedErrorMessage) + suite.assertAllExpectations() + }) } -// TestLookupTransactionErrorMessageByIndex_HappyPath tests lookup of a transaction error message by index. -// In a happy path if it wasn't found in the cache, it has to be fetched from the execution node, otherwise served from the cache. -// If the transaction has not failed, the error message must be empty. +// TestLookupTransactionErrorMessageByIndex_HappyPath verifies the lookup of a transaction error message +// by block ID and transaction index. +// It tests two cases: +// 1. Happy path where the error message is fetched from the EN if it is not found in the cache. +// 2. Happy path where the error message is served from the storage database if it exists. func (suite *Suite) TestLookupTransactionErrorMessageByIndex_HappyPath() { block := unittest.BlockFixture() blockId := block.ID() @@ -435,154 +533,186 @@ func (suite *Suite) TestLookupTransactionErrorMessageByIndex_HappyPath() { failedTxId := failedTx.ID() failedTxIndex := rand.Uint32() - suite.transactionResults.On("ByBlockIDTransactionIndex", blockId, failedTxIndex). - Return(&flow.LightTransactionResult{ - TransactionID: failedTxId, - Failed: true, - ComputationUsed: 0, - }, nil).Twice() - + // Setup mock receipts and execution node identities. _, fixedENIDs := suite.setupReceipts(&block) suite.state.On("Final").Return(suite.snapshot, nil).Maybe() suite.snapshot.On("Identities", mock.Anything).Return(fixedENIDs, nil) - // create a mock connection factory - connFactory := connectionmock.NewConnectionFactory(suite.T()) - connFactory.On("GetExecutionAPIClient", mock.Anything).Return(suite.execClient, &mockCloser{}, nil) - - // create a mock index reporter - reporter := syncmock.NewIndexReporter(suite.T()) - reporter.On("LowestIndexedHeight").Return(block.Header.Height, nil) - reporter.On("HighestIndexedHeight").Return(block.Header.Height+10, nil) + suite.fixedExecutionNodeIDs = fixedENIDs.NodeIDs() params := suite.defaultBackendParams() + params.TxResultErrorMessages = suite.txErrorMessages - // the connection factory should be used to get the execution node client - params.ConnFactory = connFactory - params.FixedExecutionNodeIDs = fixedENIDs.NodeIDs().Strings() - - params.TxResultsIndex = index.NewTransactionResultsIndex(index.NewReporter(), suite.transactionResults) - err := params.TxResultsIndex.Initialize(reporter) - suite.Require().NoError(err) - - backend, err := New(params) - suite.Require().NoError(err) - - expectedErrorMsg := "some error" - - exeEventReq := &execproto.GetTransactionErrorMessageByIndexRequest{ - BlockId: blockId[:], - Index: failedTxIndex, - } - - exeEventResp := &execproto.GetTransactionErrorMessageResponse{ - TransactionId: failedTxId[:], - ErrorMessage: expectedErrorMsg, - } - - suite.execClient.On("GetTransactionErrorMessageByIndex", mock.Anything, exeEventReq).Return(exeEventResp, nil).Once() - - errMsg, err := backend.LookupErrorMessageByIndex(context.Background(), blockId, block.Header.Height, failedTxIndex) - suite.Require().NoError(err) - suite.Require().Equal(expectedErrorMsg, errMsg) - - // ensure the transaction error message is cached after retrieval; we do this by mocking the grpc call - // only once - errMsg, err = backend.LookupErrorMessageByIndex(context.Background(), blockId, block.Header.Height, failedTxIndex) - suite.Require().NoError(err) - suite.Require().Equal(expectedErrorMsg, errMsg) - suite.assertAllExpectations() -} - -// TestLookupTransactionErrorMessageByIndex_UnknownTransaction tests lookup of a transaction error message by index, -// when a transaction result has not been synced yet, in this case nothing we can do but return an error. -func (suite *Suite) TestLookupTransactionErrorMessageByIndex_UnknownTransaction() { - block := unittest.BlockFixture() - blockId := block.ID() - failedTxIndex := rand.Uint32() - - suite.transactionResults.On("ByBlockIDTransactionIndex", blockId, failedTxIndex). - Return(nil, storage.ErrNotFound).Once() + // Test case: transaction error message is fetched from the EN. + suite.Run("happy path from EN", func() { + // the connection factory should be used to get the execution node client + params.ConnFactory = suite.setupConnectionFactory() - // create a mock index reporter - reporter := syncmock.NewIndexReporter(suite.T()) - reporter.On("LowestIndexedHeight").Return(block.Header.Height, nil) - reporter.On("HighestIndexedHeight").Return(block.Header.Height+10, nil) + // Mock the cache lookup for the transaction error message, returning "not found". + suite.txErrorMessages.On("ByBlockIDTransactionIndex", blockId, failedTxIndex). + Return(nil, storage.ErrNotFound).Once() - params := suite.defaultBackendParams() + backend, err := New(params) + suite.Require().NoError(err) - params.TxResultsIndex = index.NewTransactionResultsIndex(index.NewReporter(), suite.transactionResults) - err := params.TxResultsIndex.Initialize(reporter) - suite.Require().NoError(err) + // Mock the execution node API call to fetch the error message. + exeEventReq := &execproto.GetTransactionErrorMessageByIndexRequest{ + BlockId: blockId[:], + Index: failedTxIndex, + } + exeEventResp := &execproto.GetTransactionErrorMessageResponse{ + TransactionId: failedTxId[:], + ErrorMessage: expectedErrorMsg, + } + suite.execClient.On("GetTransactionErrorMessageByIndex", mock.Anything, exeEventReq).Return(exeEventResp, nil).Once() - backend, err := New(params) - suite.Require().NoError(err) + // Perform the lookup and assert that the error message is retrieved correctly. + errMsg, err := backend.LookupErrorMessageByIndex(context.Background(), blockId, block.Header.Height, failedTxIndex) + suite.Require().NoError(err) + suite.Require().Equal(expectedErrorMsg, errMsg) + suite.assertAllExpectations() + }) - errMsg, err := backend.LookupErrorMessageByIndex(context.Background(), blockId, block.Header.Height, failedTxIndex) - suite.Require().Error(err) - suite.Require().Equal(codes.NotFound, status.Code(err)) - suite.Require().Empty(errMsg) + // Test case: transaction error message is fetched from the storage database. + suite.Run("happy path from storage db", func() { + backend, err := New(params) + suite.Require().NoError(err) - suite.assertAllExpectations() + // Mock the cache lookup for the transaction error message, returning a stored result. + suite.txErrorMessages.On("ByBlockIDTransactionIndex", blockId, failedTxIndex). + Return(&flow.TransactionResultErrorMessage{ + TransactionID: failedTxId, + ErrorMessage: expectedErrorMsg, + Index: failedTxIndex, + ExecutorID: unittest.IdentifierFixture(), + }, nil).Once() + + // Perform the lookup and assert that the error message is retrieved correctly from storage. + errMsg, err := backend.LookupErrorMessageByIndex(context.Background(), blockId, block.Header.Height, failedTxIndex) + suite.Require().NoError(err) + suite.Require().Equal(expectedErrorMsg, errMsg) + suite.assertAllExpectations() + }) } -// TestLookupTransactionErrorMessageByIndex_FailedToFetch tests lookup of a transaction error message by index, -// when a transaction result is not in the cache and needs to be fetched from EN, but the EN fails to return it. +// TestLookupTransactionErrorMessageByIndex_FailedToFetch verifies the behavior of looking up a transaction error message by index +// when the error message is not in the cache, and fetching it from the EN fails. +// It tests three cases: +// 1. The transaction is not found in the transaction results, leading to a "NotFound" error. +// 2. The transaction result is not failed, and the error message is empty. +// 3. The transaction result is failed, and the error message "failed" are returned. func (suite *Suite) TestLookupTransactionErrorMessageByIndex_FailedToFetch() { block := unittest.BlockFixture() blockId := block.ID() + failedTxIndex := rand.Uint32() failedTx := unittest.TransactionFixture() failedTxId := failedTx.ID() - failedTxIndex := rand.Uint32() - - suite.transactionResults.On("ByBlockIDTransactionIndex", blockId, failedTxIndex). - Return(&flow.LightTransactionResult{ - TransactionID: failedTxId, - Failed: true, - ComputationUsed: 0, - }, nil).Once() + // Setup mock receipts and execution node identities. _, fixedENIDs := suite.setupReceipts(&block) suite.state.On("Final").Return(suite.snapshot, nil).Maybe() suite.snapshot.On("Identities", mock.Anything).Return(fixedENIDs, nil) - // create a mock connection factory + // Create a mock connection factory connFactory := connectionmock.NewConnectionFactory(suite.T()) - connFactory.On("GetExecutionAPIClient", mock.Anything).Return(suite.execClient, &mockCloser{}, nil) + connFactory.On("GetExecutionAPIClient", mock.Anything).Return(suite.execClient, &mocks.MockCloser{}, nil) - // create a mock index reporter + // Create a mock index reporter reporter := syncmock.NewIndexReporter(suite.T()) reporter.On("LowestIndexedHeight").Return(block.Header.Height, nil) reporter.On("HighestIndexedHeight").Return(block.Header.Height+10, nil) + suite.fixedExecutionNodeIDs = fixedENIDs.NodeIDs() + params := suite.defaultBackendParams() // the connection factory should be used to get the execution node client params.ConnFactory = connFactory - params.FixedExecutionNodeIDs = fixedENIDs.NodeIDs().Strings() - + // Initialize the transaction results index with the mock reporter. params.TxResultsIndex = index.NewTransactionResultsIndex(index.NewReporter(), suite.transactionResults) err := params.TxResultsIndex.Initialize(reporter) suite.Require().NoError(err) + params.TxResultErrorMessages = suite.txErrorMessages + backend, err := New(params) suite.Require().NoError(err) - // lookup should try each of the 2 ENs in fixedENIDs - suite.execClient.On("GetTransactionErrorMessageByIndex", mock.Anything, mock.Anything).Return(nil, - status.Error(codes.Unavailable, "")).Twice() + // Test case: failed to fetch from EN, transaction is unknown. + suite.Run("failed to fetch from EN, unknown tx", func() { + // lookup should try each of the 2 ENs in fixedENIDs + suite.execClient.On("GetTransactionErrorMessageByIndex", mock.Anything, mock.Anything).Return(nil, + status.Error(codes.Unavailable, "")).Twice() + + // Setup mock that the transaction and tx error message is not found in the storage. + suite.txErrorMessages.On("ByBlockIDTransactionIndex", blockId, failedTxIndex). + Return(nil, storage.ErrNotFound).Once() + suite.transactionResults.On("ByBlockIDTransactionIndex", blockId, failedTxIndex). + Return(nil, storage.ErrNotFound).Once() + + // Perform the lookup and expect a "NotFound" error with an empty error message. + errMsg, err := backend.LookupErrorMessageByIndex(context.Background(), blockId, block.Header.Height, failedTxIndex) + suite.Require().Error(err) + suite.Require().Equal(codes.NotFound, status.Code(err)) + suite.Require().Empty(errMsg) + suite.assertAllExpectations() + }) - errMsg, err := backend.LookupErrorMessageByIndex(context.Background(), blockId, block.Header.Height, failedTxIndex) - suite.Require().Error(err) - suite.Require().Equal(codes.Unavailable, status.Code(err)) - suite.Require().Empty(errMsg) + // Test case: failed to fetch from EN, but the transaction result is not failed. + suite.Run("failed to fetch from EN, tx result is not failed", func() { + // lookup should try each of the 2 ENs in fixedENIDs + suite.execClient.On("GetTransactionErrorMessageByIndex", mock.Anything, mock.Anything).Return(nil, + status.Error(codes.Unavailable, "")).Twice() + + // Setup mock that the transaction error message is not found in storage. + suite.txErrorMessages.On("ByBlockIDTransactionIndex", blockId, failedTxIndex). + Return(nil, storage.ErrNotFound).Once() + + // Setup mock that the transaction result exists and is not failed. + suite.transactionResults.On("ByBlockIDTransactionIndex", blockId, failedTxIndex). + Return(&flow.LightTransactionResult{ + TransactionID: failedTxId, + Failed: false, + ComputationUsed: 0, + }, nil).Once() + + // Perform the lookup and expect no error and an empty error message. + errMsg, err := backend.LookupErrorMessageByIndex(context.Background(), blockId, block.Header.Height, failedTxIndex) + suite.Require().NoError(err) + suite.Require().Empty(errMsg) + suite.assertAllExpectations() + }) - suite.assertAllExpectations() + // Test case: failed to fetch from EN, but the transaction result is failed. + suite.Run("failed to fetch from EN, tx result is failed", func() { + // lookup should try each of the 2 ENs in fixedENIDs + suite.execClient.On("GetTransactionErrorMessageByIndex", mock.Anything, mock.Anything).Return(nil, + status.Error(codes.Unavailable, "")).Twice() + + // Setup mock that the transaction error message is not found in storage. + suite.txErrorMessages.On("ByBlockIDTransactionIndex", blockId, failedTxIndex). + Return(nil, storage.ErrNotFound).Once() + + // Setup mock that the transaction result exists and is failed. + suite.transactionResults.On("ByBlockIDTransactionIndex", blockId, failedTxIndex). + Return(&flow.LightTransactionResult{ + TransactionID: failedTxId, + Failed: true, + ComputationUsed: 0, + }, nil).Once() + + // Perform the lookup and expect the failed error message to be returned. + errMsg, err := backend.LookupErrorMessageByIndex(context.Background(), blockId, block.Header.Height, failedTxIndex) + suite.Require().NoError(err) + suite.Require().Equal(errMsg, FailedErrorMessage) + suite.assertAllExpectations() + }) } -// TestLookupTransactionErrorMessages_HappyPath tests lookup of a transaction error messages by block ID. -// In a happy path, it has to be fetched from the execution node if there are no cached results. -// All fetched transactions have to be cached for future calls. -func (suite *Suite) TestLookupTransactionErrorMessages_HappyPath() { +// TestLookupTransactionErrorMessagesByBlockID_HappyPath verifies the lookup of transaction error messages by block ID. +// It tests two cases: +// 1. Happy path where the error messages are fetched from the EN if they are not found in the cache. +// 2. Happy path where the error messages are served from the storage database if they exist. +func (suite *Suite) TestLookupTransactionErrorMessagesByBlockID_HappyPath() { block := unittest.BlockFixture() blockId := block.ID() @@ -595,211 +725,227 @@ func (suite *Suite) TestLookupTransactionErrorMessages_HappyPath() { }) } - suite.transactionResults.On("ByBlockID", blockId). - Return(resultsByBlockID, nil).Twice() - _, fixedENIDs := suite.setupReceipts(&block) suite.state.On("Final").Return(suite.snapshot, nil).Maybe() suite.snapshot.On("Identities", mock.Anything).Return(fixedENIDs, nil) - // create a mock connection factory - connFactory := connectionmock.NewConnectionFactory(suite.T()) - connFactory.On("GetExecutionAPIClient", mock.Anything).Return(suite.execClient, &mockCloser{}, nil) - - // create a mock index reporter - reporter := syncmock.NewIndexReporter(suite.T()) - reporter.On("LowestIndexedHeight").Return(block.Header.Height, nil) - reporter.On("HighestIndexedHeight").Return(block.Header.Height+10, nil) + suite.fixedExecutionNodeIDs = fixedENIDs.NodeIDs() params := suite.defaultBackendParams() + params.TxResultErrorMessages = suite.txErrorMessages - // the connection factory should be used to get the execution node client - params.ConnFactory = connFactory - params.FixedExecutionNodeIDs = fixedENIDs.NodeIDs().Strings() - - params.TxResultsIndex = index.NewTransactionResultsIndex(index.NewReporter(), suite.transactionResults) - err := params.TxResultsIndex.Initialize(reporter) - suite.Require().NoError(err) + // Test case: transaction error messages is fetched from the EN. + suite.Run("happy path from EN", func() { + // the connection factory should be used to get the execution node client + params.ConnFactory = suite.setupConnectionFactory() - backend, err := New(params) - suite.Require().NoError(err) + // Mock the cache lookup for the transaction error messages, returning "not found". + suite.txErrorMessages.On("ByBlockID", blockId). + Return(nil, storage.ErrNotFound).Once() - expectedErrorMsg := "some error" + backend, err := New(params) + suite.Require().NoError(err) - exeEventReq := &execproto.GetTransactionErrorMessagesByBlockIDRequest{ - BlockId: blockId[:], - } + // Mock the execution node API call to fetch the error messages. + exeEventReq := &execproto.GetTransactionErrorMessagesByBlockIDRequest{ + BlockId: blockId[:], + } + exeErrMessagesResp := &execproto.GetTransactionErrorMessagesResponse{} + for _, result := range resultsByBlockID { + r := result + if r.Failed { + errMsg := fmt.Sprintf("%s.%s", expectedErrorMsg, r.TransactionID) + exeErrMessagesResp.Results = append(exeErrMessagesResp.Results, &execproto.GetTransactionErrorMessagesResponse_Result{ + TransactionId: r.TransactionID[:], + ErrorMessage: errMsg, + }) + } + } + suite.execClient.On("GetTransactionErrorMessagesByBlockID", mock.Anything, exeEventReq). + Return(exeErrMessagesResp, nil). + Once() - exeEventResp := &execproto.GetTransactionErrorMessagesResponse{} - for _, result := range resultsByBlockID { - r := result - if r.Failed { - errMsg := fmt.Sprintf("%s.%s", expectedErrorMsg, r.TransactionID) - exeEventResp.Results = append(exeEventResp.Results, &execproto.GetTransactionErrorMessagesResponse_Result{ - TransactionId: r.TransactionID[:], - ErrorMessage: errMsg, - }) + // Perform the lookup and assert that the error message is retrieved correctly. + errMessages, err := backend.LookupErrorMessagesByBlockID(context.Background(), blockId, block.Header.Height) + suite.Require().NoError(err) + suite.Require().Len(errMessages, len(exeErrMessagesResp.Results)) + for _, expectedResult := range exeErrMessagesResp.Results { + errMsg, ok := errMessages[convert.MessageToIdentifier(expectedResult.TransactionId)] + suite.Require().True(ok) + suite.Assert().Equal(expectedResult.ErrorMessage, errMsg) } - } + suite.assertAllExpectations() + }) - suite.execClient.On("GetTransactionErrorMessagesByBlockID", mock.Anything, exeEventReq). - Return(exeEventResp, nil). - Once() + // Test case: transaction error messages is fetched from the storage database. + suite.Run("happy path from storage db", func() { + backend, err := New(params) + suite.Require().NoError(err) - errMessages, err := backend.LookupErrorMessagesByBlockID(context.Background(), blockId, block.Header.Height) - suite.Require().NoError(err) - suite.Require().Len(errMessages, len(exeEventResp.Results)) - for _, expectedResult := range exeEventResp.Results { - errMsg, ok := errMessages[convert.MessageToIdentifier(expectedResult.TransactionId)] - suite.Require().True(ok) - suite.Assert().Equal(expectedResult.ErrorMessage, errMsg) - } + // Mock the cache lookup for the transaction error messages, returning a stored result. + var txErrorMessages []flow.TransactionResultErrorMessage + for i, result := range resultsByBlockID { + if result.Failed { + errMsg := fmt.Sprintf("%s.%s", expectedErrorMsg, result.TransactionID) + + txErrorMessages = append(txErrorMessages, + flow.TransactionResultErrorMessage{ + TransactionID: result.TransactionID, + ErrorMessage: errMsg, + Index: uint32(i), + ExecutorID: unittest.IdentifierFixture(), + }) + } + } + suite.txErrorMessages.On("ByBlockID", blockId). + Return(txErrorMessages, nil).Once() - // ensure the transaction error message is cached after retrieval; we do this by mocking the grpc call - // only once - errMessages, err = backend.LookupErrorMessagesByBlockID(context.Background(), blockId, block.Header.Height) - suite.Require().NoError(err) - suite.Require().Len(errMessages, len(exeEventResp.Results)) - for _, expectedResult := range exeEventResp.Results { - errMsg, ok := errMessages[convert.MessageToIdentifier(expectedResult.TransactionId)] - suite.Require().True(ok) - suite.Assert().Equal(expectedResult.ErrorMessage, errMsg) - } - suite.assertAllExpectations() + // Perform the lookup and assert that the error message is retrieved correctly from storage. + errMessages, err := backend.LookupErrorMessagesByBlockID(context.Background(), blockId, block.Header.Height) + suite.Require().NoError(err) + suite.Require().Len(errMessages, len(txErrorMessages)) + for _, expected := range txErrorMessages { + errMsg, ok := errMessages[expected.TransactionID] + suite.Require().True(ok) + suite.Assert().Equal(expected.ErrorMessage, errMsg) + } + suite.assertAllExpectations() + }) } -// TestLookupTransactionErrorMessages_HappyPath_NoFailedTxns tests lookup of a transaction error messages by block ID. -// In a happy path where a block with no failed txns is requested. We don't want to perform an RPC call in this case. -func (suite *Suite) TestLookupTransactionErrorMessages_HappyPath_NoFailedTxns() { +// TestLookupTransactionErrorMessagesByBlockID_FailedToFetch tests lookup of a transaction error messages by block ID, +// when a transaction result is not in the cache and needs to be fetched from EN, but the EN fails to return it. +// It tests three cases: +// 1. The transaction is not found in the transaction results, leading to a "NotFound" error. +// 2. The transaction result is not failed, and the error message is empty. +// 3. The transaction result is failed, and the error message "failed" are returned. +func (suite *Suite) TestLookupTransactionErrorMessagesByBlockID_FailedToFetch() { block := unittest.BlockFixture() blockId := block.ID() - resultsByBlockID := []flow.LightTransactionResult{ - { - TransactionID: unittest.IdentifierFixture(), - Failed: false, - ComputationUsed: 0, - }, - { - TransactionID: unittest.IdentifierFixture(), - Failed: false, - ComputationUsed: 0, - }, - } - - suite.transactionResults.On("ByBlockID", blockId). - Return(resultsByBlockID, nil).Once() + // Setup mock receipts and execution node identities. + _, fixedENIDs := suite.setupReceipts(&block) + suite.state.On("Final").Return(suite.snapshot, nil).Maybe() + suite.snapshot.On("Identities", mock.Anything).Return(fixedENIDs, nil) - // create a mock index reporter + // Create a mock index reporter reporter := syncmock.NewIndexReporter(suite.T()) reporter.On("LowestIndexedHeight").Return(block.Header.Height, nil) reporter.On("HighestIndexedHeight").Return(block.Header.Height+10, nil) - params := suite.defaultBackendParams() - - params.TxResultsIndex = index.NewTransactionResultsIndex(index.NewReporter(), suite.transactionResults) - err := params.TxResultsIndex.Initialize(reporter) - suite.Require().NoError(err) - - backend, err := New(params) - suite.Require().NoError(err) - - errMessages, err := backend.LookupErrorMessagesByBlockID(context.Background(), blockId, block.Header.Height) - suite.Require().NoError(err) - suite.Require().Empty(errMessages) - suite.assertAllExpectations() -} - -// TestLookupTransactionErrorMessages_UnknownTransaction tests lookup of a transaction error messages by block ID, -// when a transaction results for block has not been synced yet, in this case nothing we can do but return an error. -func (suite *Suite) TestLookupTransactionErrorMessages_UnknownTransaction() { - block := unittest.BlockFixture() - blockId := block.ID() - - suite.transactionResults.On("ByBlockID", blockId). - Return(nil, storage.ErrNotFound).Once() - - // create a mock index reporter - reporter := syncmock.NewIndexReporter(suite.T()) - reporter.On("LowestIndexedHeight").Return(block.Header.Height, nil) - reporter.On("HighestIndexedHeight").Return(block.Header.Height+10, nil) + suite.fixedExecutionNodeIDs = fixedENIDs.NodeIDs() params := suite.defaultBackendParams() - + // the connection factory should be used to get the execution node client + params.ConnFactory = suite.setupConnectionFactory() + // Initialize the transaction results index with the mock reporter. params.TxResultsIndex = index.NewTransactionResultsIndex(index.NewReporter(), suite.transactionResults) err := params.TxResultsIndex.Initialize(reporter) suite.Require().NoError(err) + params.TxResultErrorMessages = suite.txErrorMessages + backend, err := New(params) suite.Require().NoError(err) - errMsg, err := backend.LookupErrorMessagesByBlockID(context.Background(), blockId, block.Header.Height) - suite.Require().Error(err) - suite.Require().Equal(codes.NotFound, status.Code(err)) - suite.Require().Empty(errMsg) + // Test case: failed to fetch from EN, transaction is unknown. + suite.Run("failed to fetch from EN, unknown tx", func() { + // lookup should try each of the 2 ENs in fixedENIDs + suite.execClient.On("GetTransactionErrorMessagesByBlockID", mock.Anything, mock.Anything).Return(nil, + status.Error(codes.Unavailable, "")).Twice() - suite.assertAllExpectations() -} - -// TestLookupTransactionErrorMessages_FailedToFetch tests lookup of a transaction error messages by block ID, -// when a transaction result is not in the cache and needs to be fetched from EN, but the EN fails to return it. -func (suite *Suite) TestLookupTransactionErrorMessages_FailedToFetch() { - block := unittest.BlockFixture() - blockId := block.ID() + // Setup mock that the transaction and tx error messages is not found in the storage. + suite.txErrorMessages.On("ByBlockID", blockId). + Return(nil, storage.ErrNotFound).Once() + suite.transactionResults.On("ByBlockID", blockId). + Return(nil, storage.ErrNotFound).Once() - resultsByBlockID := []flow.LightTransactionResult{ - { - TransactionID: unittest.IdentifierFixture(), - Failed: true, - ComputationUsed: 0, - }, - { - TransactionID: unittest.IdentifierFixture(), - Failed: true, - ComputationUsed: 0, - }, - } - - suite.transactionResults.On("ByBlockID", blockId). - Return(resultsByBlockID, nil).Once() - - _, fixedENIDs := suite.setupReceipts(&block) - suite.state.On("Final").Return(suite.snapshot, nil).Maybe() - suite.snapshot.On("Identities", mock.Anything).Return(fixedENIDs, nil) - - // create a mock connection factory - connFactory := connectionmock.NewConnectionFactory(suite.T()) - connFactory.On("GetExecutionAPIClient", mock.Anything).Return(suite.execClient, &mockCloser{}, nil) - - // create a mock index reporter - reporter := syncmock.NewIndexReporter(suite.T()) - reporter.On("LowestIndexedHeight").Return(block.Header.Height, nil) - reporter.On("HighestIndexedHeight").Return(block.Header.Height+10, nil) + // Perform the lookup and expect a "NotFound" error with an empty error message. + errMsg, err := backend.LookupErrorMessagesByBlockID(context.Background(), blockId, block.Header.Height) + suite.Require().Error(err) + suite.Require().Equal(codes.NotFound, status.Code(err)) + suite.Require().Empty(errMsg) + suite.assertAllExpectations() + }) - params := suite.defaultBackendParams() - // the connection factory should be used to get the execution node client - params.ConnFactory = connFactory - params.FixedExecutionNodeIDs = fixedENIDs.NodeIDs().Strings() + // Test case: failed to fetch from EN, but the transaction result is not failed. + suite.Run("failed to fetch from EN, tx result is not failed", func() { + // lookup should try each of the 2 ENs in fixedENIDs + suite.execClient.On("GetTransactionErrorMessagesByBlockID", mock.Anything, mock.Anything).Return(nil, + status.Error(codes.Unavailable, "")).Twice() + + // Setup mock that the transaction error message is not found in storage. + suite.txErrorMessages.On("ByBlockID", blockId). + Return(nil, storage.ErrNotFound).Once() + + // Setup mock that the transaction results exists and is not failed. + suite.transactionResults.On("ByBlockID", blockId). + Return([]flow.LightTransactionResult{ + { + TransactionID: unittest.IdentifierFixture(), + Failed: false, + ComputationUsed: 0, + }, + { + TransactionID: unittest.IdentifierFixture(), + Failed: false, + ComputationUsed: 0, + }, + }, nil).Once() + + // Perform the lookup and expect no error and an empty error messages. + errMsg, err := backend.LookupErrorMessagesByBlockID(context.Background(), blockId, block.Header.Height) + suite.Require().NoError(err) + suite.Require().Empty(errMsg) + suite.assertAllExpectations() + }) - params.TxResultsIndex = index.NewTransactionResultsIndex(index.NewReporter(), suite.transactionResults) - err := params.TxResultsIndex.Initialize(reporter) - suite.Require().NoError(err) + // Test case: failed to fetch from EN, but the transaction result is failed. + suite.Run("failed to fetch from EN, tx result is failed", func() { + failedResultsByBlockID := []flow.LightTransactionResult{ + { + TransactionID: unittest.IdentifierFixture(), + Failed: true, + ComputationUsed: 0, + }, + { + TransactionID: unittest.IdentifierFixture(), + Failed: true, + ComputationUsed: 0, + }, + } - backend, err := New(params) - suite.Require().NoError(err) + // lookup should try each of the 2 ENs in fixedENIDs + suite.execClient.On("GetTransactionErrorMessagesByBlockID", mock.Anything, mock.Anything).Return(nil, + status.Error(codes.Unavailable, "")).Twice() - // pretend the first transaction has been cached, but there are multiple failed txns so still a request has to be made. - backend.txErrorMessagesCache.Add(resultsByBlockID[0].TransactionID, "some error") + // Setup mock that the transaction error messages is not found in storage. + suite.txErrorMessages.On("ByBlockID", blockId). + Return(nil, storage.ErrNotFound).Once() - suite.execClient.On("GetTransactionErrorMessagesByBlockID", mock.Anything, mock.Anything).Return(nil, - status.Error(codes.Unavailable, "")).Twice() + // Setup mock that the transaction results exists and is failed. + suite.transactionResults.On("ByBlockID", blockId). + Return(failedResultsByBlockID, nil).Once() - errMsg, err := backend.LookupErrorMessagesByBlockID(context.Background(), blockId, block.Header.Height) - suite.Require().Error(err) - suite.Require().Equal(codes.Unavailable, status.Code(err)) - suite.Require().Empty(errMsg) + // Setup mock expected the transaction error messages after retrieving the failed result. + expectedTxErrorMessages := make(map[flow.Identifier]string) + for _, result := range failedResultsByBlockID { + if result.Failed { + expectedTxErrorMessages[result.TransactionID] = FailedErrorMessage + } + } - suite.assertAllExpectations() + // Perform the lookup and expect the failed error messages to be returned. + errMsg, err := backend.LookupErrorMessagesByBlockID(context.Background(), blockId, block.Header.Height) + suite.Require().NoError(err) + suite.Require().Len(errMsg, len(expectedTxErrorMessages)) + for txID, expectedMessage := range expectedTxErrorMessages { + actualMessage, ok := errMsg[txID] + suite.Require().True(ok) + suite.Assert().Equal(expectedMessage, actualMessage) + } + suite.assertAllExpectations() + }) } // TestGetSystemTransaction_HappyPath tests that GetSystemTransaction call returns system chunk transaction. @@ -851,13 +997,9 @@ func (suite *Suite) TestGetSystemTransactionResult_HappyPath() { On("ByBlockID", block.ID()). Return(flow.ExecutionReceiptList{receipt1}, nil) - // create a mock connection factory - connFactory := connectionmock.NewConnectionFactory(suite.T()) - connFactory.On("GetExecutionAPIClient", mock.Anything).Return(suite.execClient, &mockCloser{}, nil) - // the connection factory should be used to get the execution node client params := suite.defaultBackendParams() - params.ConnFactory = connFactory + params.ConnFactory = suite.setupConnectionFactory() exeEventReq := &execproto.GetTransactionsByBlockIDRequest{ BlockId: blockID[:], @@ -1046,13 +1188,9 @@ func (suite *Suite) TestGetSystemTransactionResult_FailedEncodingConversion() { On("ByBlockID", block.ID()). Return(flow.ExecutionReceiptList{receipt1}, nil) - // create a mock connection factory - connFactory := connectionmock.NewConnectionFactory(suite.T()) - connFactory.On("GetExecutionAPIClient", mock.Anything).Return(suite.execClient, &mockCloser{}, nil) - // the connection factory should be used to get the execution node client params := suite.defaultBackendParams() - params.ConnFactory = connFactory + params.ConnFactory = suite.setupConnectionFactory() exeEventReq := &execproto.GetTransactionsByBlockIDRequest{ BlockId: blockID[:], @@ -1175,10 +1313,6 @@ func (suite *Suite) TestTransactionResultFromStorage() { suite.snapshot.On("Identities", mock.Anything).Return(fixedENIDs, nil) suite.snapshot.On("Head", mock.Anything).Return(block.Header, nil) - // create a mock connection factory - connFactory := connectionmock.NewConnectionFactory(suite.T()) - connFactory.On("GetExecutionAPIClient", mock.Anything).Return(suite.execClient, &mockCloser{}, nil) - // create a mock index reporter reporter := syncmock.NewIndexReporter(suite.T()) reporter.On("LowestIndexedHeight").Return(block.Header.Height, nil) @@ -1188,13 +1322,13 @@ func (suite *Suite) TestTransactionResultFromStorage() { err := indexReporter.Initialize(reporter) suite.Require().NoError(err) + suite.fixedExecutionNodeIDs = fixedENIDs.NodeIDs() + // Set up the backend parameters and the backend instance params := suite.defaultBackendParams() // the connection factory should be used to get the execution node client - params.ConnFactory = connFactory - params.FixedExecutionNodeIDs = fixedENIDs.NodeIDs().Strings() + params.ConnFactory = suite.setupConnectionFactory() params.TxResultQueryMode = IndexQueryModeLocalOnly - params.EventsIndex = index.NewEventsIndex(indexReporter, suite.events) params.TxResultsIndex = index.NewTransactionResultsIndex(indexReporter, suite.transactionResults) @@ -1266,10 +1400,6 @@ func (suite *Suite) TestTransactionByIndexFromStorage() { suite.snapshot.On("Identities", mock.Anything).Return(fixedENIDs, nil) suite.snapshot.On("Head", mock.Anything).Return(block.Header, nil) - // Create a mock connection factory - connFactory := connectionmock.NewConnectionFactory(suite.T()) - connFactory.On("GetExecutionAPIClient", mock.Anything).Return(suite.execClient, &mockCloser{}, nil) - // create a mock index reporter reporter := syncmock.NewIndexReporter(suite.T()) reporter.On("LowestIndexedHeight").Return(block.Header.Height, nil) @@ -1279,13 +1409,13 @@ func (suite *Suite) TestTransactionByIndexFromStorage() { err := indexReporter.Initialize(reporter) suite.Require().NoError(err) + suite.fixedExecutionNodeIDs = fixedENIDs.NodeIDs() + // Set up the backend parameters and the backend instance params := suite.defaultBackendParams() // the connection factory should be used to get the execution node client - params.ConnFactory = connFactory - params.FixedExecutionNodeIDs = fixedENIDs.NodeIDs().Strings() + params.ConnFactory = suite.setupConnectionFactory() params.TxResultQueryMode = IndexQueryModeLocalOnly - params.EventsIndex = index.NewEventsIndex(indexReporter, suite.events) params.TxResultsIndex = index.NewTransactionResultsIndex(indexReporter, suite.transactionResults) @@ -1363,10 +1493,6 @@ func (suite *Suite) TestTransactionResultsByBlockIDFromStorage() { suite.snapshot.On("Identities", mock.Anything).Return(fixedENIDs, nil) suite.snapshot.On("Head", mock.Anything).Return(block.Header, nil) - // create a mock connection factory - connFactory := connectionmock.NewConnectionFactory(suite.T()) - connFactory.On("GetExecutionAPIClient", mock.Anything).Return(suite.execClient, &mockCloser{}, nil) - // create a mock index reporter reporter := syncmock.NewIndexReporter(suite.T()) reporter.On("LowestIndexedHeight").Return(block.Header.Height, nil) @@ -1376,15 +1502,14 @@ func (suite *Suite) TestTransactionResultsByBlockIDFromStorage() { err := indexReporter.Initialize(reporter) suite.Require().NoError(err) + suite.fixedExecutionNodeIDs = fixedENIDs.NodeIDs() + // Set up the state and snapshot mocks and the backend instance params := suite.defaultBackendParams() // the connection factory should be used to get the execution node client - params.ConnFactory = connFactory - params.FixedExecutionNodeIDs = fixedENIDs.NodeIDs().Strings() - + params.ConnFactory = suite.setupConnectionFactory() params.EventsIndex = index.NewEventsIndex(indexReporter, suite.events) params.TxResultsIndex = index.NewTransactionResultsIndex(indexReporter, suite.transactionResults) - params.TxResultQueryMode = IndexQueryModeLocalOnly backend, err := New(params) diff --git a/engine/access/rpc/backend/node_selector.go b/engine/access/rpc/backend/node_selector.go index c7d2ada5fb4..4aab3a89ca5 100644 --- a/engine/access/rpc/backend/node_selector.go +++ b/engine/access/rpc/backend/node_selector.go @@ -3,12 +3,10 @@ package backend import ( "fmt" + commonrpc "github.com/onflow/flow-go/engine/common/rpc" "github.com/onflow/flow-go/model/flow" ) -// maxNodesCnt is the maximum number of nodes that will be contacted to complete an API request. -const maxNodesCnt = 3 - // NodeSelector is an interface that represents the ability to select node identities that the access node is trying to reach. // It encapsulates the internal logic of node selection and provides a way to change implementations for different types // of nodes. Implementations of this interface should define the Next method, which returns the next node identity to be @@ -26,13 +24,26 @@ type NodeSelectorFactory struct { circuitBreakerEnabled bool } +// NewNodeSelectorFactory creates a new instance of NodeSelectorFactory with the provided circuit breaker configuration. +// +// When `circuitBreakerEnabled` is set to true, nodes will be selected using a pseudo-random sampling mechanism and picked in-order. +// When set to false, nodes will be selected in the order they are proposed, without any changes. +// +// Parameters: +// - circuitBreakerEnabled: A boolean that controls whether the circuit breaker is enabled for node selection. +func NewNodeSelectorFactory(circuitBreakerEnabled bool) *NodeSelectorFactory { + return &NodeSelectorFactory{ + circuitBreakerEnabled: circuitBreakerEnabled, + } +} + // SelectNodes selects the configured number of node identities from the provided list of nodes // and returns the node selector to iterate through them. func (n *NodeSelectorFactory) SelectNodes(nodes flow.IdentitySkeletonList) (NodeSelector, error) { var err error // If the circuit breaker is disabled, the legacy logic should be used, which selects only a specified number of nodes. if !n.circuitBreakerEnabled { - nodes, err = nodes.Sample(maxNodesCnt) + nodes, err = nodes.Sample(commonrpc.MaxNodesCnt) if err != nil { return nil, fmt.Errorf("sampling failed: %w", err) } diff --git a/engine/access/rpc/backend/transactions_local_data_provider.go b/engine/access/rpc/backend/transactions_local_data_provider.go index f68fb35ed4b..921580452ec 100644 --- a/engine/access/rpc/backend/transactions_local_data_provider.go +++ b/engine/access/rpc/backend/transactions_local_data_provider.go @@ -31,18 +31,16 @@ type TransactionErrorMessage interface { // Expected errors during normal operation: // - InsufficientExecutionReceipts - found insufficient receipts for given block ID. // - status.Error - remote GRPC call to EN has failed. - LookupErrorMessageByTransactionID(ctx context.Context, blockID flow.Identifier, transactionID flow.Identifier) (string, error) + LookupErrorMessageByTransactionID(ctx context.Context, blockID flow.Identifier, height uint64, transactionID flow.Identifier) (string, error) // LookupErrorMessageByIndex is a function type for getting transaction error message by index. // Expected errors during normal operation: - // - status.Error[codes.NotFound] - transaction result for given block ID and tx index is not available. // - InsufficientExecutionReceipts - found insufficient receipts for given block ID. // - status.Error - remote GRPC call to EN has failed. LookupErrorMessageByIndex(ctx context.Context, blockID flow.Identifier, height uint64, index uint32) (string, error) // LookupErrorMessagesByBlockID is a function type for getting transaction error messages by block ID. // Expected errors during normal operation: - // - status.Error[codes.NotFound] - transaction results for given block ID are not available. // - InsufficientExecutionReceipts - found insufficient receipts for given block ID. // - status.Error - remote GRPC call to EN has failed. LookupErrorMessagesByBlockID(ctx context.Context, blockID flow.Identifier, height uint64) (map[flow.Identifier]string, error) @@ -84,7 +82,7 @@ func (t *TransactionsLocalDataProvider) GetTransactionResultFromStorage( var txErrorMessage string var txStatusCode uint = 0 if txResult.Failed { - txErrorMessage, err = t.txErrorMessages.LookupErrorMessageByTransactionID(ctx, blockID, transactionID) + txErrorMessage, err = t.txErrorMessages.LookupErrorMessageByTransactionID(ctx, blockID, block.Header.Height, transactionID) if err != nil { return nil, err } diff --git a/engine/common/rpc/execution_node_identities_provider.go b/engine/common/rpc/execution_node_identities_provider.go new file mode 100644 index 00000000000..0a56c53a08a --- /dev/null +++ b/engine/common/rpc/execution_node_identities_provider.go @@ -0,0 +1,300 @@ +package rpc + +import ( + "context" + "fmt" + "time" + + "github.com/rs/zerolog" + + "github.com/onflow/flow-go/model/flow" + "github.com/onflow/flow-go/model/flow/filter" + "github.com/onflow/flow-go/state/protocol" + "github.com/onflow/flow-go/storage" +) + +// minExecutionNodesCnt is the minimum number of execution nodes expected to have sent the execution receipt for a block +const minExecutionNodesCnt = 2 + +// maxAttemptsForExecutionReceipt is the maximum number of attempts to find execution receipts for a given block ID +const maxAttemptsForExecutionReceipt = 3 + +// MaxNodesCnt is the maximum number of nodes that will be contacted to complete an API request. +const MaxNodesCnt = 3 + +func IdentifierList(ids []string) (flow.IdentifierList, error) { + idList := make(flow.IdentifierList, len(ids)) + for i, idStr := range ids { + id, err := flow.HexStringToIdentifier(idStr) + if err != nil { + return nil, fmt.Errorf("failed to convert node id string %s to Flow Identifier: %w", id, err) + } + idList[i] = id + } + return idList, nil +} + +// ExecutionNodeIdentitiesProvider is a container for elements required to retrieve +// execution node identities for a given block ID. +type ExecutionNodeIdentitiesProvider struct { + log zerolog.Logger + + executionReceipts storage.ExecutionReceipts + state protocol.State + + preferredENIdentifiers flow.IdentifierList + fixedENIdentifiers flow.IdentifierList +} + +// NewExecutionNodeIdentitiesProvider creates and returns a new instance of +// ExecutionNodeIdentitiesProvider. +// +// Parameters: +// - log: The logger to use for logging. +// - state: The protocol state used for retrieving block information. +// - executionReceipts: A storage.ExecutionReceipts object that contains the execution receipts +// for blocks. +// - preferredENIdentifiers: A flow.IdentifierList of preferred execution node identifiers that +// are prioritized during selection. +// - fixedENIdentifiers: A flow.IdentifierList of fixed execution node identifiers that are +// always considered if available. +// +// No error returns are expected during normal operations. +func NewExecutionNodeIdentitiesProvider( + log zerolog.Logger, + state protocol.State, + executionReceipts storage.ExecutionReceipts, + preferredENIdentifiers flow.IdentifierList, + fixedENIdentifiers flow.IdentifierList, +) *ExecutionNodeIdentitiesProvider { + return &ExecutionNodeIdentitiesProvider{ + log: log, + executionReceipts: executionReceipts, + state: state, + preferredENIdentifiers: preferredENIdentifiers, + fixedENIdentifiers: fixedENIdentifiers, + } +} + +// ExecutionNodesForBlockID returns upto maxNodesCnt number of randomly chosen execution node identities +// which have executed the given block ID. +// If no such execution node is found, an InsufficientExecutionReceipts error is returned. +func (e *ExecutionNodeIdentitiesProvider) ExecutionNodesForBlockID( + ctx context.Context, + blockID flow.Identifier, +) (flow.IdentitySkeletonList, error) { + var ( + executorIDs flow.IdentifierList + err error + ) + + // check if the block ID is of the root block. If it is then don't look for execution receipts since they + // will not be present for the root block. + rootBlock := e.state.Params().FinalizedRoot() + + if rootBlock.ID() == blockID { + executorIdentities, err := e.state.Final().Identities(filter.HasRole[flow.Identity](flow.RoleExecution)) + if err != nil { + return nil, fmt.Errorf("failed to retreive execution IDs for block ID %v: %w", blockID, err) + } + executorIDs = executorIdentities.NodeIDs() + } else { + // try to find at least minExecutionNodesCnt execution node ids from the execution receipts for the given blockID + for attempt := 0; attempt < maxAttemptsForExecutionReceipt; attempt++ { + executorIDs, err = e.findAllExecutionNodes(blockID) + if err != nil { + return nil, err + } + + if len(executorIDs) >= minExecutionNodesCnt { + break + } + + // log the attempt + e.log.Debug().Int("attempt", attempt).Int("max_attempt", maxAttemptsForExecutionReceipt). + Int("execution_receipts_found", len(executorIDs)). + Str("block_id", blockID.String()). + Msg("insufficient execution receipts") + + // if one or less execution receipts may have been received then re-query + // in the hope that more might have been received by now + + select { + case <-ctx.Done(): + return nil, ctx.Err() + case <-time.After(100 * time.Millisecond << time.Duration(attempt)): + // retry after an exponential backoff + } + } + + receiptCnt := len(executorIDs) + // if less than minExecutionNodesCnt execution receipts have been received so far, then return random ENs + if receiptCnt < minExecutionNodesCnt { + newExecutorIDs, err := e.state.AtBlockID(blockID).Identities(filter.HasRole[flow.Identity](flow.RoleExecution)) + if err != nil { + return nil, fmt.Errorf("failed to retreive execution IDs for block ID %v: %w", blockID, err) + } + executorIDs = newExecutorIDs.NodeIDs() + } + } + + // choose from the preferred or fixed execution nodes + subsetENs, err := e.chooseExecutionNodes(executorIDs) + if err != nil { + return nil, fmt.Errorf("failed to retreive execution IDs for block ID %v: %w", blockID, err) + } + + if len(subsetENs) == 0 { + return nil, fmt.Errorf("no matching execution node found for block ID %v", blockID) + } + + return subsetENs, nil +} + +// findAllExecutionNodes find all the execution nodes ids from the execution receipts that have been received for the +// given blockID +func (e *ExecutionNodeIdentitiesProvider) findAllExecutionNodes( + blockID flow.Identifier, +) (flow.IdentifierList, error) { + // lookup the receipt's storage with the block ID + allReceipts, err := e.executionReceipts.ByBlockID(blockID) + if err != nil { + return nil, fmt.Errorf("failed to retreive execution receipts for block ID %v: %w", blockID, err) + } + + executionResultMetaList := make(flow.ExecutionReceiptMetaList, 0, len(allReceipts)) + for _, r := range allReceipts { + executionResultMetaList = append(executionResultMetaList, r.Meta()) + } + executionResultGroupedMetaList := executionResultMetaList.GroupByResultID() + + // maximum number of matching receipts found so far for any execution result id + maxMatchedReceiptCnt := 0 + // execution result id key for the highest number of matching receipts in the identicalReceipts map + var maxMatchedReceiptResultID flow.Identifier + + // find the largest list of receipts which have the same result ID + for resultID, executionReceiptList := range executionResultGroupedMetaList { + currentMatchedReceiptCnt := executionReceiptList.Size() + if currentMatchedReceiptCnt > maxMatchedReceiptCnt { + maxMatchedReceiptCnt = currentMatchedReceiptCnt + maxMatchedReceiptResultID = resultID + } + } + + // if there are more than one execution result for the same block ID, log as error + if executionResultGroupedMetaList.NumberGroups() > 1 { + identicalReceiptsStr := fmt.Sprintf("%v", flow.GetIDs(allReceipts)) + e.log.Error(). + Str("block_id", blockID.String()). + Str("execution_receipts", identicalReceiptsStr). + Msg("execution receipt mismatch") + } + + // pick the largest list of matching receipts + matchingReceiptMetaList := executionResultGroupedMetaList.GetGroup(maxMatchedReceiptResultID) + + metaReceiptGroupedByExecutorID := matchingReceiptMetaList.GroupByExecutorID() + + // collect all unique execution node ids from the receipts + var executorIDs flow.IdentifierList + for executorID := range metaReceiptGroupedByExecutorID { + executorIDs = append(executorIDs, executorID) + } + + return executorIDs, nil +} + +// chooseExecutionNodes finds the subset of execution nodes defined in the identity table by first +// choosing the preferred execution nodes which have executed the transaction. If no such preferred +// execution nodes are found, then the fixed execution nodes defined in the identity table are returned +// If neither preferred nor fixed nodes are defined, then all execution node matching the executor IDs are returned. +// e.g. If execution nodes in identity table are {1,2,3,4}, preferred ENs are defined as {2,3,4} +// and the executor IDs is {1,2,3}, then {2, 3} is returned as the chosen subset of ENs +func (e *ExecutionNodeIdentitiesProvider) chooseExecutionNodes( + executorIDs flow.IdentifierList, +) (flow.IdentitySkeletonList, error) { + allENs, err := e.state.Final().Identities(filter.HasRole[flow.Identity](flow.RoleExecution)) + if err != nil { + return nil, fmt.Errorf("failed to retrieve all execution IDs: %w", err) + } + + // choose from preferred EN IDs + if len(e.preferredENIdentifiers) > 0 { + chosenIDs := e.ChooseFromPreferredENIDs(allENs, executorIDs) + return chosenIDs.ToSkeleton(), nil + } + + // if no preferred EN ID is found, then choose from the fixed EN IDs + if len(e.fixedENIdentifiers) > 0 { + // choose fixed ENs which have executed the transaction + chosenIDs := allENs.Filter(filter.And( + filter.HasNodeID[flow.Identity](e.fixedENIdentifiers...), + filter.HasNodeID[flow.Identity](executorIDs...), + )) + if len(chosenIDs) > 0 { + return chosenIDs.ToSkeleton(), nil + } + // if no such ENs are found, then just choose all fixed ENs + chosenIDs = allENs.Filter(filter.HasNodeID[flow.Identity](e.fixedENIdentifiers...)) + return chosenIDs.ToSkeleton(), nil + } + + // if no preferred or fixed ENs have been specified, then return all executor IDs i.e., no preference at all + return allENs.Filter(filter.HasNodeID[flow.Identity](executorIDs...)).ToSkeleton(), nil +} + +// ChooseFromPreferredENIDs finds the subset of execution nodes if preferred execution nodes are defined. +// If preferredENIdentifiers is set and there are less than maxNodesCnt nodes selected, than the list is padded up to +// maxNodesCnt nodes using the following order: +// 1. Use any EN with a receipt. +// 2. Use any preferred node not already selected. +// 3. Use any EN not already selected. +func (e *ExecutionNodeIdentitiesProvider) ChooseFromPreferredENIDs( + allENs flow.IdentityList, + executorIDs flow.IdentifierList, +) flow.IdentityList { + var chosenIDs flow.IdentityList + + // filter for both preferred and executor IDs + chosenIDs = allENs.Filter(filter.And( + filter.HasNodeID[flow.Identity](e.preferredENIdentifiers...), + filter.HasNodeID[flow.Identity](executorIDs...), + )) + + if len(chosenIDs) >= MaxNodesCnt { + return chosenIDs + } + + // function to add nodes to chosenIDs if they are not already included + addIfNotExists := func(candidates flow.IdentityList) { + for _, en := range candidates { + _, exists := chosenIDs.ByNodeID(en.NodeID) + if !exists { + chosenIDs = append(chosenIDs, en) + if len(chosenIDs) >= MaxNodesCnt { + return + } + } + } + } + + // add any EN with a receipt + receiptENs := allENs.Filter(filter.HasNodeID[flow.Identity](executorIDs...)) + addIfNotExists(receiptENs) + if len(chosenIDs) >= MaxNodesCnt { + return chosenIDs + } + + // add any preferred node not already selected + preferredENs := allENs.Filter(filter.HasNodeID[flow.Identity](e.preferredENIdentifiers...)) + addIfNotExists(preferredENs) + if len(chosenIDs) >= MaxNodesCnt { + return chosenIDs + } + + // add any EN not already selected + addIfNotExists(allENs) + + return chosenIDs +} diff --git a/engine/common/rpc/execution_node_identities_provider_test.go b/engine/common/rpc/execution_node_identities_provider_test.go new file mode 100644 index 00000000000..eb23faf34a6 --- /dev/null +++ b/engine/common/rpc/execution_node_identities_provider_test.go @@ -0,0 +1,263 @@ +package rpc_test + +import ( + "context" + "testing" + + "github.com/rs/zerolog" + "github.com/stretchr/testify/mock" + "github.com/stretchr/testify/require" + "github.com/stretchr/testify/suite" + + "github.com/onflow/flow-go/engine/access/rpc/backend" + commonrpc "github.com/onflow/flow-go/engine/common/rpc" + "github.com/onflow/flow-go/model/flow" + protocol "github.com/onflow/flow-go/state/protocol/mock" + storagemock "github.com/onflow/flow-go/storage/mock" + "github.com/onflow/flow-go/utils/unittest" +) + +type Suite struct { + suite.Suite + + state *protocol.State + snapshot *protocol.Snapshot + log zerolog.Logger + + receipts *storagemock.ExecutionReceipts +} + +func TestHandler(t *testing.T) { + suite.Run(t, new(Suite)) +} + +func (suite *Suite) SetupTest() { + suite.log = zerolog.New(zerolog.NewConsoleWriter()) + suite.state = new(protocol.State) + suite.snapshot = new(protocol.Snapshot) + suite.receipts = new(storagemock.ExecutionReceipts) + + header := unittest.BlockHeaderFixture() + params := new(protocol.Params) + params.On("FinalizedRoot").Return(header, nil) + suite.state.On("Params").Return(params) +} + +// TestExecutionNodesForBlockID tests the ExecutionNodesForBlockID used for serving all API calls +// that need to talk to an execution node. +func (suite *Suite) TestExecutionNodesForBlockID() { + totalReceipts := 5 + + block := unittest.BlockFixture() + + // generate one execution node identities for each receipt assuming that each ER is generated by a unique exec node + allExecutionNodes := unittest.IdentityListFixture(totalReceipts, unittest.WithRole(flow.RoleExecution)) + + // one execution result for all receipts for this block + executionResult := unittest.ExecutionResultFixture() + + // generate execution receipts + receipts := make(flow.ExecutionReceiptList, totalReceipts) + for j := 0; j < totalReceipts; j++ { + r := unittest.ReceiptForBlockFixture(&block) + r.ExecutorID = allExecutionNodes[j].NodeID + er := *executionResult + r.ExecutionResult = er + receipts[j] = r + } + + currentAttempt := 0 + attempt1Receipts, attempt2Receipts, attempt3Receipts := receipts, receipts, receipts + + // setup receipts storage mock to return different list of receipts on each call + suite.receipts. + On("ByBlockID", block.ID()).Return( + func(id flow.Identifier) flow.ExecutionReceiptList { + switch currentAttempt { + case 0: + currentAttempt++ + return attempt1Receipts + case 1: + currentAttempt++ + return attempt2Receipts + default: + currentAttempt = 0 + return attempt3Receipts + } + }, + func(id flow.Identifier) error { return nil }) + + suite.snapshot.On("Identities", mock.Anything).Return( + func(filter flow.IdentityFilter[flow.Identity]) flow.IdentityList { + // apply the filter passed in to the list of all the execution nodes + return allExecutionNodes.Filter(filter) + }, + func(flow.IdentityFilter[flow.Identity]) error { return nil }) + suite.state.On("Final").Return(suite.snapshot, nil).Maybe() + + var preferredENIdentifiers flow.IdentifierList + var fixedENIdentifiers flow.IdentifierList + + testExecutionNodesForBlockID := func(preferredENs, fixedENs, expectedENs flow.IdentityList) { + + if preferredENs != nil { + preferredENIdentifiers = preferredENs.NodeIDs() + } + if fixedENs != nil { + fixedENIdentifiers = fixedENs.NodeIDs() + } + + if expectedENs == nil { + expectedENs = flow.IdentityList{} + } + + execNodeIdentitiesProvider := commonrpc.NewExecutionNodeIdentitiesProvider( + suite.log, + suite.state, + suite.receipts, + preferredENIdentifiers, + fixedENIdentifiers, + ) + + allExecNodes, err := execNodeIdentitiesProvider.ExecutionNodesForBlockID(context.Background(), block.ID()) + require.NoError(suite.T(), err) + + execNodeSelectorFactory := backend.NewNodeSelectorFactory(false) + execSelector, err := execNodeSelectorFactory.SelectNodes(allExecNodes) + require.NoError(suite.T(), err) + + actualList := flow.IdentitySkeletonList{} + for actual := execSelector.Next(); actual != nil; actual = execSelector.Next() { + actualList = append(actualList, actual) + } + + { + expectedENs := expectedENs.ToSkeleton() + if len(expectedENs) > commonrpc.MaxNodesCnt { + for _, actual := range actualList { + require.Contains(suite.T(), expectedENs, actual) + } + } else { + require.ElementsMatch(suite.T(), actualList, expectedENs) + } + } + } + // if we don't find sufficient receipts, ExecutionNodesForBlockID should return a list of random ENs + suite.Run("insufficient receipts return random ENs in State", func() { + // return no receipts at all attempts + attempt1Receipts = flow.ExecutionReceiptList{} + attempt2Receipts = flow.ExecutionReceiptList{} + attempt3Receipts = flow.ExecutionReceiptList{} + suite.state.On("AtBlockID", mock.Anything).Return(suite.snapshot) + + execNodeIdentitiesProvider := commonrpc.NewExecutionNodeIdentitiesProvider( + suite.log, + suite.state, + suite.receipts, + flow.IdentifierList{}, + flow.IdentifierList{}, + ) + + allExecNodes, err := execNodeIdentitiesProvider.ExecutionNodesForBlockID(context.Background(), block.ID()) + require.NoError(suite.T(), err) + + execNodeSelectorFactory := backend.NewNodeSelectorFactory(false) + execSelector, err := execNodeSelectorFactory.SelectNodes(allExecNodes) + require.NoError(suite.T(), err) + + actualList := flow.IdentitySkeletonList{} + for actual := execSelector.Next(); actual != nil; actual = execSelector.Next() { + actualList = append(actualList, actual) + } + + require.Equal(suite.T(), len(actualList), commonrpc.MaxNodesCnt) + }) + + // if no preferred or fixed ENs are specified, the ExecutionNodesForBlockID function should + // return the exe node list without a filter + suite.Run("no preferred or fixed ENs", func() { + testExecutionNodesForBlockID(nil, nil, allExecutionNodes) + }) + // if only fixed ENs are specified, the ExecutionNodesForBlockID function should + // return the fixed ENs list + suite.Run("two fixed ENs with zero preferred EN", func() { + // mark the first two ENs as fixed + fixedENs := allExecutionNodes[0:2] + expectedList := fixedENs + testExecutionNodesForBlockID(nil, fixedENs, expectedList) + }) + // if only preferred ENs are specified, the ExecutionNodesForBlockID function should + // return the preferred ENs list + suite.Run("two preferred ENs with zero fixed EN", func() { + // mark the first two ENs as preferred + preferredENs := allExecutionNodes[0:2] + expectedList := allExecutionNodes[0:commonrpc.MaxNodesCnt] + testExecutionNodesForBlockID(preferredENs, nil, expectedList) + }) + // if both are specified, the ExecutionNodesForBlockID function should + // return the preferred ENs list + suite.Run("four fixed ENs of which two are preferred ENs", func() { + // mark the first four ENs as fixed + fixedENs := allExecutionNodes[0:5] + // mark the first two of the fixed ENs as preferred ENs + preferredENs := fixedENs[0:2] + expectedList := fixedENs[0:commonrpc.MaxNodesCnt] + testExecutionNodesForBlockID(preferredENs, fixedENs, expectedList) + }) + // if both are specified, but the preferred ENs don't match the ExecutorIDs in the ER, + // the ExecutionNodesForBlockID function should return the fixed ENs list + suite.Run("four fixed ENs of which two are preferred ENs but have not generated the ER", func() { + // mark the first two ENs as fixed + fixedENs := allExecutionNodes[0:2] + // specify two ENs not specified in the ERs as preferred + preferredENs := unittest.IdentityListFixture(2, unittest.WithRole(flow.RoleExecution)) + // add one more node ID besides of the fixed ENs list cause expected length of the list should be maxNodesCnt + expectedList := append(fixedENs, allExecutionNodes[2]) + testExecutionNodesForBlockID(preferredENs, fixedENs, expectedList) + }) + // if execution receipts are not yet available, the ExecutionNodesForBlockID function should retry twice + suite.Run("retry execution receipt query", func() { + // on first attempt, no execution receipts are available + attempt1Receipts = flow.ExecutionReceiptList{} + // on second attempt ony one is available + attempt2Receipts = flow.ExecutionReceiptList{receipts[0]} + // on third attempt all receipts are available + attempt3Receipts = receipts + currentAttempt = 0 + // mark the first two ENs as preferred + preferredENs := allExecutionNodes[0:2] + expectedList := allExecutionNodes[0:commonrpc.MaxNodesCnt] + testExecutionNodesForBlockID(preferredENs, nil, expectedList) + }) + // if preferredENIdentifiers was set and there are less than maxNodesCnt nodes selected than check the order + // of adding ENs ids + suite.Run("add nodes in the correct order", func() { + // mark the first EN as preferred + preferredENIdentifiers = allExecutionNodes[0:1].NodeIDs() + // mark the fourth EN with receipt + executorIDs := allExecutionNodes[3:4].NodeIDs() + + receiptNodes := allExecutionNodes[3:4] // any EN with a receipt + preferredNodes := allExecutionNodes[0:1] // preferred EN node not already selected + additionalNode := allExecutionNodes[1:2] // any EN not already selected + + expectedOrder := flow.IdentityList{ + receiptNodes[0], + preferredNodes[0], + additionalNode[0], + } + + execNodeIdentitiesProvider := commonrpc.NewExecutionNodeIdentitiesProvider( + suite.log, + suite.state, + suite.receipts, + preferredENIdentifiers, + flow.IdentifierList{}, + ) + + chosenIDs := execNodeIdentitiesProvider.ChooseFromPreferredENIDs(allExecutionNodes, executorIDs) + + require.ElementsMatch(suite.T(), chosenIDs, expectedOrder) + require.Equal(suite.T(), len(chosenIDs), commonrpc.MaxNodesCnt) + }) +} diff --git a/integration/tests/access/cohort3/access_store_tx_error_messages_test.go b/integration/tests/access/cohort3/access_store_tx_error_messages_test.go new file mode 100644 index 00000000000..9697010e97c --- /dev/null +++ b/integration/tests/access/cohort3/access_store_tx_error_messages_test.go @@ -0,0 +1,200 @@ +package cohort3 + +import ( + "context" + "fmt" + "testing" + "time" + + "github.com/rs/zerolog" + "github.com/stretchr/testify/require" + "github.com/stretchr/testify/suite" + + sdk "github.com/onflow/flow-go-sdk" + sdkcrypto "github.com/onflow/flow-go-sdk/crypto" + "github.com/onflow/flow-go-sdk/templates" + + "github.com/onflow/flow-go/integration/convert" + "github.com/onflow/flow-go/integration/testnet" + "github.com/onflow/flow-go/integration/tests/lib" + "github.com/onflow/flow-go/model/flow" + "github.com/onflow/flow-go/module/metrics" + "github.com/onflow/flow-go/storage/badger" +) + +const maxReceiptHeightMetric = "access_ingestion_max_receipt_height" + +func TestAccessStoreTxErrorMessages(t *testing.T) { + suite.Run(t, new(AccessStoreTxErrorMessagesSuite)) +} + +// ObserverIndexerEnabledSuite tests the observer with the indexer enabled. +// It uses ObserverSuite as a base to reuse the test cases that need to be run for any observer variation. +type AccessStoreTxErrorMessagesSuite struct { + suite.Suite + + log zerolog.Logger + + // root context for the current test + ctx context.Context + cancel context.CancelFunc + + net *testnet.FlowNetwork +} + +func (s *AccessStoreTxErrorMessagesSuite) TearDownTest() { + s.log.Info().Msg("================> Start TearDownTest") + s.net.Remove() + s.cancel() + s.log.Info().Msg("================> Finish TearDownTest") +} + +// SetupTest sets up the test suite by starting the network and preparing create, send and waiting for sealed transaction. +// The access are started with correct parameters and store transaction error messages. +func (s *AccessStoreTxErrorMessagesSuite) SetupTest() { + defaultAccess := testnet.NewNodeConfig( + flow.RoleAccess, + testnet.WithLogLevel(zerolog.FatalLevel), + ) + + storeTxAccess := testnet.NewNodeConfig( + flow.RoleAccess, + testnet.WithLogLevel(zerolog.InfoLevel), + testnet.WithAdditionalFlagf("--store-tx-result-error-messages=true"), + testnet.WithMetricsServer(), + ) + + consensusConfigs := []func(config *testnet.NodeConfig){ + // `cruise-ctl-fallback-proposal-duration` is set to 250ms instead to of 100ms + // to purposely slow down the block rate. This is needed since the crypto module + // update providing faster BLS operations. + testnet.WithAdditionalFlag("--cruise-ctl-fallback-proposal-duration=250ms"), + testnet.WithAdditionalFlagf("--required-verification-seal-approvals=%d", 1), + testnet.WithAdditionalFlagf("--required-construction-seal-approvals=%d", 1), + testnet.WithLogLevel(zerolog.FatalLevel), + } + + nodeConfigs := []testnet.NodeConfig{ + testnet.NewNodeConfig(flow.RoleCollection, testnet.WithLogLevel(zerolog.FatalLevel)), + testnet.NewNodeConfig(flow.RoleCollection, testnet.WithLogLevel(zerolog.FatalLevel)), + testnet.NewNodeConfig(flow.RoleExecution, testnet.WithLogLevel(zerolog.FatalLevel)), + testnet.NewNodeConfig(flow.RoleExecution, testnet.WithLogLevel(zerolog.FatalLevel)), + testnet.NewNodeConfig(flow.RoleConsensus, consensusConfigs...), + testnet.NewNodeConfig(flow.RoleConsensus, consensusConfigs...), + testnet.NewNodeConfig(flow.RoleConsensus, consensusConfigs...), + testnet.NewNodeConfig(flow.RoleVerification, testnet.WithLogLevel(zerolog.FatalLevel)), + + defaultAccess, + storeTxAccess, + } + + // prepare the network + conf := testnet.NewNetworkConfig("access_store_tx_error_messages_test", nodeConfigs) + s.net = testnet.PrepareFlowNetwork(s.T(), conf, flow.Localnet) + + // start the network + s.ctx, s.cancel = context.WithCancel(context.Background()) + + s.net.Start(s.ctx) +} + +// TestAccessStoreTxErrorMessages verifies that transaction result error messages +// are stored correctly in the database by sending a transaction, generating an error, +// and checking if the error message is properly stored and retrieved from the database. +func (s *AccessStoreTxErrorMessagesSuite) TestAccessStoreTxErrorMessages() { + // Create and send a transaction that will result in an error. + txResult := s.createAndSendTxWithTxError() + + txBlockID := convert.IDFromSDK(txResult.BlockID) + txID := convert.IDFromSDK(txResult.TransactionID) + expectedTxResultErrorMessage := txResult.Error.Error() + + accessContainerName := "access_2" + + // Wait until execution receipts are handled, transaction error messages are stored. + s.Eventually(func() bool { + value, err := s.getMaxReceiptHeight(accessContainerName) + return err == nil && value > txResult.BlockHeight + }, 60*time.Second, 1*time.Second) + + // Stop the network containers before checking the results. + s.net.StopContainers() + + // Get the access node and open the protocol DB. + accessNode := s.net.ContainerByName(accessContainerName) + // setup storage objects needed to get the execution data id + anDB, err := accessNode.DB() + require.NoError(s.T(), err, "could not open db") + + metrics := metrics.NewNoopCollector() + anTxErrorMessages := badger.NewTransactionResultErrorMessages(metrics, anDB, badger.DefaultCacheSize) + + // Fetch the stored error message by block ID and transaction ID. + errMsgResult, err := anTxErrorMessages.ByBlockIDTransactionID(txBlockID, txID) + s.Require().NoError(err) + + // Verify that the error message retrieved matches the expected values. + s.Require().Equal(txID, errMsgResult.TransactionID) + s.Require().Equal(expectedTxResultErrorMessage, errMsgResult.ErrorMessage) +} + +// createAndSendTxWithTxError creates and sends a transaction that will result in an error. +// This function creates a new account, causing an error during execution. +func (s *AccessStoreTxErrorMessagesSuite) createAndSendTxWithTxError() *sdk.TransactionResult { + // prepare environment to create a new account + serviceAccountClient, err := s.net.ContainerByName(testnet.PrimaryAN).TestnetClient() + s.Require().NoError(err) + + latestBlockID, err := serviceAccountClient.GetLatestBlockID(s.ctx) + s.Require().NoError(err) + + // create new account to deploy Counter to + accountPrivateKey := lib.RandomPrivateKey() + + accountKey := sdk.NewAccountKey(). + FromPrivateKey(accountPrivateKey). + SetHashAlgo(sdkcrypto.SHA3_256). + SetWeight(sdk.AccountKeyWeightThreshold) + + serviceAddress := sdk.Address(serviceAccountClient.Chain.ServiceAddress()) + + // Generate the account creation transaction + createAccountTx, err := templates.CreateAccount( + []*sdk.AccountKey{accountKey}, + []templates.Contract{}, serviceAddress) + s.Require().NoError(err) + + // Generate the account creation transaction + createAccountTx. + SetReferenceBlockID(sdk.Identifier(latestBlockID)). + SetProposalKey(serviceAddress, 1, serviceAccountClient.GetAndIncrementSeqNumber()). + SetPayer(serviceAddress). + SetComputeLimit(9999) + + // Sign and send the transaction. + err = serviceAccountClient.SignAndSendTransaction(s.ctx, createAccountTx) + s.Require().NoError(err) + + // Wait for the transaction to be sealed and return the transaction result. + accountCreationTxRes, err := serviceAccountClient.WaitForSealed(s.ctx, createAccountTx.ID()) + s.Require().NoError(err) + + return accountCreationTxRes +} + +// getMaxReceiptHeight retrieves the maximum receipt height for a given container by +// querying the metrics endpoint. This is used to confirm that the transaction receipts +// have been processed. +func (s *AccessStoreTxErrorMessagesSuite) getMaxReceiptHeight(containerName string) (uint64, error) { + node := s.net.ContainerByName(containerName) + metricsURL := fmt.Sprintf("http://0.0.0.0:%s/metrics", node.Port(testnet.MetricsPort)) + values := s.net.GetMetricFromContainer(s.T(), containerName, metricsURL, maxReceiptHeightMetric) + + // If no values are found in the metrics, return an error. + if len(values) == 0 { + return 0, fmt.Errorf("no values found") + } + + // Return the first value found as the max receipt height. + return uint64(values[0].GetGauge().GetValue()), nil +} diff --git a/model/flow/transaction_result.go b/model/flow/transaction_result.go index 3a327118165..4e1c0532874 100644 --- a/model/flow/transaction_result.go +++ b/model/flow/transaction_result.go @@ -52,3 +52,14 @@ type LightTransactionResult struct { // ComputationUsed is amount of computation used while executing the transaction. ComputationUsed uint64 } + +type TransactionResultErrorMessage struct { + // TransactionID is the ID of the transaction this result error was emitted from. + TransactionID Identifier + // Index is the index of the transaction this result error was emitted from. + Index uint32 + // ErrorMessage contains the error message of any error that may have occurred when the transaction was executed. + ErrorMessage string + // Executor node ID of the execution node that the message was received from. + ExecutorID Identifier +} diff --git a/module/metrics/labels.go b/module/metrics/labels.go index 82260ca3c5d..20b66ad7d68 100644 --- a/module/metrics/labels.go +++ b/module/metrics/labels.go @@ -109,28 +109,30 @@ const ( ResourceNetworkingUnicastDialConfigCache = "unicast_dial_config_cache" ResourceNetworkingGossipsubDuplicateMessagesTrackerCache = "gossipsub_duplicate_messages_tracker_cache" - ResourceFollowerPendingBlocksCache = "follower_pending_block_cache" // follower engine - ResourceFollowerLoopCertifiedBlocksChannel = "follower_loop_certified_blocks_channel" // follower loop, certified blocks buffered channel - ResourceClusterBlockProposalQueue = "cluster_compliance_proposal_queue" // collection node, compliance engine - ResourceTransactionIngestQueue = "ingest_transaction_queue" // collection node, ingest engine - ResourceBeaconKey = "beacon-key" // consensus node, DKG engine - ResourceDKGMessage = "dkg_private_message" // consensus, DKG messaging engine - ResourceApprovalQueue = "sealing_approval_queue" // consensus node, sealing engine - ResourceReceiptQueue = "sealing_receipt_queue" // consensus node, sealing engine - ResourceApprovalResponseQueue = "sealing_approval_response_queue" // consensus node, sealing engine - ResourceBlockResponseQueue = "compliance_block_response_queue" // consensus node, compliance engine - ResourceBlockProposalQueue = "compliance_proposal_queue" // consensus node, compliance engine - ResourceBlockVoteQueue = "vote_aggregator_queue" // consensus/collection node, vote aggregator - ResourceTimeoutObjectQueue = "timeout_aggregator_queue" // consensus/collection node, timeout aggregator - ResourceCollectionGuaranteesQueue = "ingestion_col_guarantee_queue" // consensus node, ingestion engine - ResourceChunkDataPack = "chunk_data_pack" // execution node - ResourceChunkDataPackRequests = "chunk_data_pack_request" // execution node - ResourceEvents = "events" // execution node - ResourceServiceEvents = "service_events" // execution node - ResourceTransactionResults = "transaction_results" // execution node - ResourceTransactionResultIndices = "transaction_result_indices" // execution node - ResourceTransactionResultByBlock = "transaction_result_by_block" // execution node - ResourceExecutionDataCache = "execution_data_cache" // access node + ResourceFollowerPendingBlocksCache = "follower_pending_block_cache" // follower engine + ResourceFollowerLoopCertifiedBlocksChannel = "follower_loop_certified_blocks_channel" // follower loop, certified blocks buffered channel + ResourceClusterBlockProposalQueue = "cluster_compliance_proposal_queue" // collection node, compliance engine + ResourceTransactionIngestQueue = "ingest_transaction_queue" // collection node, ingest engine + ResourceBeaconKey = "beacon-key" // consensus node, DKG engine + ResourceDKGMessage = "dkg_private_message" // consensus, DKG messaging engine + ResourceApprovalQueue = "sealing_approval_queue" // consensus node, sealing engine + ResourceReceiptQueue = "sealing_receipt_queue" // consensus node, sealing engine + ResourceApprovalResponseQueue = "sealing_approval_response_queue" // consensus node, sealing engine + ResourceBlockResponseQueue = "compliance_block_response_queue" // consensus node, compliance engine + ResourceBlockProposalQueue = "compliance_proposal_queue" // consensus node, compliance engine + ResourceBlockVoteQueue = "vote_aggregator_queue" // consensus/collection node, vote aggregator + ResourceTimeoutObjectQueue = "timeout_aggregator_queue" // consensus/collection node, timeout aggregator + ResourceCollectionGuaranteesQueue = "ingestion_col_guarantee_queue" // consensus node, ingestion engine + ResourceChunkDataPack = "chunk_data_pack" // execution node + ResourceChunkDataPackRequests = "chunk_data_pack_request" // execution node + ResourceEvents = "events" // execution node + ResourceServiceEvents = "service_events" // execution node + ResourceTransactionResults = "transaction_results" // execution node + ResourceTransactionResultIndices = "transaction_result_indices" // execution node + ResourceTransactionResultErrorMessages = "transaction_result_error_messages" // execution node + ResourceTransactionResultErrorMessagesIndices = "transaction_result_error_messages_indices" // execution node + ResourceTransactionResultByBlock = "transaction_result_by_block" // execution node + ResourceExecutionDataCache = "execution_data_cache" // access node ) const ( diff --git a/storage/all.go b/storage/all.go index 2d0075aa8be..26bb89bd454 100644 --- a/storage/all.go +++ b/storage/all.go @@ -2,26 +2,27 @@ package storage // All includes all the storage modules type All struct { - Headers Headers - Guarantees Guarantees - Seals Seals - Index Index - Payloads Payloads - Blocks Blocks - QuorumCertificates QuorumCertificates - Setups EpochSetups - EpochCommits EpochCommits - Results ExecutionResults - Receipts ExecutionReceipts - ChunkDataPacks ChunkDataPacks - Commits Commits - Transactions Transactions - LightTransactionResults LightTransactionResults - TransactionResults TransactionResults - Collections Collections - Events Events - EpochProtocolStateEntries EpochProtocolStateEntries - ProtocolKVStore ProtocolKVStore - VersionBeacons VersionBeacons - RegisterIndex RegisterIndex + Headers Headers + Guarantees Guarantees + Seals Seals + Index Index + Payloads Payloads + Blocks Blocks + QuorumCertificates QuorumCertificates + Setups EpochSetups + EpochCommits EpochCommits + Results ExecutionResults + Receipts ExecutionReceipts + ChunkDataPacks ChunkDataPacks + Commits Commits + Transactions Transactions + LightTransactionResults LightTransactionResults + TransactionResults TransactionResults + TransactionResultErrorMessages TransactionResultErrorMessages + Collections Collections + Events Events + EpochProtocolStateEntries EpochProtocolStateEntries + ProtocolKVStore ProtocolKVStore + VersionBeacons VersionBeacons + RegisterIndex RegisterIndex } diff --git a/storage/badger/operation/prefix.go b/storage/badger/operation/prefix.go index ad909faf394..4113e1fcd3f 100644 --- a/storage/badger/operation/prefix.go +++ b/storage/badger/operation/prefix.go @@ -89,20 +89,22 @@ const ( codeJobQueuePointer = 72 // legacy codes (should be cleaned up) - codeChunkDataPack = 100 - codeCommit = 101 - codeEvent = 102 - codeExecutionStateInteractions = 103 - codeTransactionResult = 104 - codeFinalizedCluster = 105 - codeServiceEvent = 106 - codeTransactionResultIndex = 107 - codeLightTransactionResult = 108 - codeLightTransactionResultIndex = 109 - codeIndexCollection = 200 - codeIndexExecutionResultByBlock = 202 - codeIndexCollectionByTransaction = 203 - codeIndexResultApprovalByChunk = 204 + codeChunkDataPack = 100 + codeCommit = 101 + codeEvent = 102 + codeExecutionStateInteractions = 103 + codeTransactionResult = 104 + codeFinalizedCluster = 105 + codeServiceEvent = 106 + codeTransactionResultIndex = 107 + codeLightTransactionResult = 108 + codeLightTransactionResultIndex = 109 + codeTransactionResultErrorMessage = 110 + codeTransactionResultErrorMessageIndex = 111 + codeIndexCollection = 200 + codeIndexExecutionResultByBlock = 202 + codeIndexCollectionByTransaction = 203 + codeIndexResultApprovalByChunk = 204 // TEMPORARY codes blockedNodeIDs = 205 // manual override for adding node IDs to list of ejected nodes, applies to networking layer only diff --git a/storage/badger/operation/transaction_results.go b/storage/badger/operation/transaction_results.go index 7d5fcf47086..c4264640364 100644 --- a/storage/badger/operation/transaction_results.go +++ b/storage/badger/operation/transaction_results.go @@ -120,3 +120,51 @@ func LookupLightTransactionResultsByBlockIDUsingIndex(blockID flow.Identifier, t return traverse(makePrefix(codeLightTransactionResultIndex, blockID), txErrIterFunc) } + +// BatchInsertTransactionResultErrorMessage inserts a transaction result error message by block ID and transaction ID +// into the database using a batch write. +func BatchInsertTransactionResultErrorMessage(blockID flow.Identifier, transactionResultErrorMessage *flow.TransactionResultErrorMessage) func(batch *badger.WriteBatch) error { + return batchWrite(makePrefix(codeTransactionResultErrorMessage, blockID, transactionResultErrorMessage.TransactionID), transactionResultErrorMessage) +} + +// BatchIndexTransactionResultErrorMessage indexes a transaction result error message by index within the block using a +// batch write. +func BatchIndexTransactionResultErrorMessage(blockID flow.Identifier, transactionResultErrorMessage *flow.TransactionResultErrorMessage) func(batch *badger.WriteBatch) error { + return batchWrite(makePrefix(codeTransactionResultErrorMessageIndex, blockID, transactionResultErrorMessage.Index), transactionResultErrorMessage) +} + +// RetrieveTransactionResultErrorMessage retrieves a transaction result error message by block ID and transaction ID. +func RetrieveTransactionResultErrorMessage(blockID flow.Identifier, transactionID flow.Identifier, transactionResultErrorMessage *flow.TransactionResultErrorMessage) func(*badger.Txn) error { + return retrieve(makePrefix(codeTransactionResultErrorMessage, blockID, transactionID), transactionResultErrorMessage) +} + +// RetrieveTransactionResultErrorMessageByIndex retrieves a transaction result error message by block ID and index. +func RetrieveTransactionResultErrorMessageByIndex(blockID flow.Identifier, txIndex uint32, transactionResultErrorMessage *flow.TransactionResultErrorMessage) func(*badger.Txn) error { + return retrieve(makePrefix(codeTransactionResultErrorMessageIndex, blockID, txIndex), transactionResultErrorMessage) +} + +// TransactionResultErrorMessagesExists checks whether tx result error messages exist in the database. +func TransactionResultErrorMessagesExists(blockID flow.Identifier, blockExists *bool) func(*badger.Txn) error { + return exists(makePrefix(codeTransactionResultErrorMessageIndex, blockID), blockExists) +} + +// LookupTransactionResultErrorMessagesByBlockIDUsingIndex retrieves all tx result error messages for a block, by using +// tx_index index. This correctly handles cases of duplicate transactions within block. +func LookupTransactionResultErrorMessagesByBlockIDUsingIndex(blockID flow.Identifier, txResultErrorMessages *[]flow.TransactionResultErrorMessage) func(*badger.Txn) error { + txErrIterFunc := func() (checkFunc, createFunc, handleFunc) { + check := func(_ []byte) bool { + return true + } + var val flow.TransactionResultErrorMessage + create := func() interface{} { + return &val + } + handle := func() error { + *txResultErrorMessages = append(*txResultErrorMessages, val) + return nil + } + return check, create, handle + } + + return traverse(makePrefix(codeTransactionResultErrorMessageIndex, blockID), txErrIterFunc) +} diff --git a/storage/badger/transaction_result_error_messages.go b/storage/badger/transaction_result_error_messages.go new file mode 100644 index 00000000000..207d472506a --- /dev/null +++ b/storage/badger/transaction_result_error_messages.go @@ -0,0 +1,208 @@ +package badger + +import ( + "fmt" + + "github.com/dgraph-io/badger/v2" + + "github.com/onflow/flow-go/model/flow" + "github.com/onflow/flow-go/module" + "github.com/onflow/flow-go/module/metrics" + "github.com/onflow/flow-go/storage" + "github.com/onflow/flow-go/storage/badger/operation" +) + +var _ storage.TransactionResultErrorMessages = (*TransactionResultErrorMessages)(nil) + +type TransactionResultErrorMessages struct { + db *badger.DB + cache *Cache[string, flow.TransactionResultErrorMessage] + indexCache *Cache[string, flow.TransactionResultErrorMessage] + blockCache *Cache[string, []flow.TransactionResultErrorMessage] +} + +func NewTransactionResultErrorMessages(collector module.CacheMetrics, db *badger.DB, transactionResultsCacheSize uint) *TransactionResultErrorMessages { + retrieve := func(key string) func(tx *badger.Txn) (flow.TransactionResultErrorMessage, error) { + var txResultErrMsg flow.TransactionResultErrorMessage + return func(tx *badger.Txn) (flow.TransactionResultErrorMessage, error) { + + blockID, txID, err := KeyToBlockIDTransactionID(key) + if err != nil { + return flow.TransactionResultErrorMessage{}, fmt.Errorf("could not convert key: %w", err) + } + + err = operation.RetrieveTransactionResultErrorMessage(blockID, txID, &txResultErrMsg)(tx) + if err != nil { + return flow.TransactionResultErrorMessage{}, handleError(err, flow.TransactionResultErrorMessage{}) + } + return txResultErrMsg, nil + } + } + retrieveIndex := func(key string) func(tx *badger.Txn) (flow.TransactionResultErrorMessage, error) { + var txResultErrMsg flow.TransactionResultErrorMessage + return func(tx *badger.Txn) (flow.TransactionResultErrorMessage, error) { + + blockID, txIndex, err := KeyToBlockIDIndex(key) + if err != nil { + return flow.TransactionResultErrorMessage{}, fmt.Errorf("could not convert index key: %w", err) + } + + err = operation.RetrieveTransactionResultErrorMessageByIndex(blockID, txIndex, &txResultErrMsg)(tx) + if err != nil { + return flow.TransactionResultErrorMessage{}, handleError(err, flow.TransactionResultErrorMessage{}) + } + return txResultErrMsg, nil + } + } + retrieveForBlock := func(key string) func(tx *badger.Txn) ([]flow.TransactionResultErrorMessage, error) { + var txResultErrMsg []flow.TransactionResultErrorMessage + return func(tx *badger.Txn) ([]flow.TransactionResultErrorMessage, error) { + + blockID, err := KeyToBlockID(key) + if err != nil { + return nil, fmt.Errorf("could not convert index key: %w", err) + } + + err = operation.LookupTransactionResultErrorMessagesByBlockIDUsingIndex(blockID, &txResultErrMsg)(tx) + if err != nil { + return nil, handleError(err, flow.TransactionResultErrorMessage{}) + } + return txResultErrMsg, nil + } + } + + return &TransactionResultErrorMessages{ + db: db, + cache: newCache[string, flow.TransactionResultErrorMessage](collector, metrics.ResourceTransactionResultErrorMessages, + withLimit[string, flow.TransactionResultErrorMessage](transactionResultsCacheSize), + withStore(noopStore[string, flow.TransactionResultErrorMessage]), + withRetrieve(retrieve), + ), + indexCache: newCache[string, flow.TransactionResultErrorMessage](collector, metrics.ResourceTransactionResultErrorMessagesIndices, + withLimit[string, flow.TransactionResultErrorMessage](transactionResultsCacheSize), + withStore(noopStore[string, flow.TransactionResultErrorMessage]), + withRetrieve(retrieveIndex), + ), + blockCache: newCache[string, []flow.TransactionResultErrorMessage](collector, metrics.ResourceTransactionResultErrorMessagesIndices, + withLimit[string, []flow.TransactionResultErrorMessage](transactionResultsCacheSize), + withStore(noopStore[string, []flow.TransactionResultErrorMessage]), + withRetrieve(retrieveForBlock), + ), + } +} + +// Store will store transaction result error messages for the given block ID. +// +// No errors are expected during normal operation. +func (t *TransactionResultErrorMessages) Store(blockID flow.Identifier, transactionResultErrorMessages []flow.TransactionResultErrorMessage) error { + batch := NewBatch(t.db) + + err := t.batchStore(blockID, transactionResultErrorMessages, batch) + if err != nil { + return err + } + + err = batch.Flush() + if err != nil { + return fmt.Errorf("cannot flush batch: %w", err) + } + + return nil +} + +// Exists returns true if transaction result error messages for the given ID have been stored. +// +// No errors are expected during normal operation. +func (t *TransactionResultErrorMessages) Exists(blockID flow.Identifier) (bool, error) { + // if the block is in the cache, return true + key := KeyFromBlockID(blockID) + if ok := t.blockCache.IsCached(key); ok { + return ok, nil + } + // otherwise, check badger store + var exists bool + err := t.db.View(operation.TransactionResultErrorMessagesExists(blockID, &exists)) + if err != nil { + return false, fmt.Errorf("could not check existence: %w", err) + } + return exists, nil +} + +// BatchStore inserts a batch of transaction result error messages into a batch +// +// No errors are expected during normal operation. +func (t *TransactionResultErrorMessages) batchStore(blockID flow.Identifier, transactionResultErrorMessages []flow.TransactionResultErrorMessage, batch storage.BatchStorage) error { + writeBatch := batch.GetWriter() + + for _, result := range transactionResultErrorMessages { + err := operation.BatchInsertTransactionResultErrorMessage(blockID, &result)(writeBatch) + if err != nil { + return fmt.Errorf("cannot batch insert tx result error message: %w", err) + } + + err = operation.BatchIndexTransactionResultErrorMessage(blockID, &result)(writeBatch) + if err != nil { + return fmt.Errorf("cannot batch index tx result error message: %w", err) + } + } + + batch.OnSucceed(func() { + for _, result := range transactionResultErrorMessages { + key := KeyFromBlockIDTransactionID(blockID, result.TransactionID) + // cache for each transaction, so that it's faster to retrieve + t.cache.Insert(key, result) + + keyIndex := KeyFromBlockIDIndex(blockID, result.Index) + t.indexCache.Insert(keyIndex, result) + } + + key := KeyFromBlockID(blockID) + t.blockCache.Insert(key, transactionResultErrorMessages) + }) + return nil +} + +// ByBlockIDTransactionID returns the transaction result error message for the given block ID and transaction ID +// +// Expected errors during normal operation: +// - `storage.ErrNotFound` if no transaction error message is known at given block and transaction id. +func (t *TransactionResultErrorMessages) ByBlockIDTransactionID(blockID flow.Identifier, transactionID flow.Identifier) (*flow.TransactionResultErrorMessage, error) { + tx := t.db.NewTransaction(false) + defer tx.Discard() + key := KeyFromBlockIDTransactionID(blockID, transactionID) + transactionResultErrorMessage, err := t.cache.Get(key)(tx) + if err != nil { + return nil, err + } + return &transactionResultErrorMessage, nil +} + +// ByBlockIDTransactionIndex returns the transaction result error message for the given blockID and transaction index +// +// Expected errors during normal operation: +// - `storage.ErrNotFound` if no transaction error message is known at given block and transaction index. +func (t *TransactionResultErrorMessages) ByBlockIDTransactionIndex(blockID flow.Identifier, txIndex uint32) (*flow.TransactionResultErrorMessage, error) { + tx := t.db.NewTransaction(false) + defer tx.Discard() + key := KeyFromBlockIDIndex(blockID, txIndex) + transactionResultErrorMessage, err := t.indexCache.Get(key)(tx) + if err != nil { + return nil, err + } + return &transactionResultErrorMessage, nil +} + +// ByBlockID gets all transaction result error messages for a block, ordered by transaction index. +// +// Expected errors during normal operation: +// - `storage.ErrNotFound` if no transaction error messages is known at given block. +func (t *TransactionResultErrorMessages) ByBlockID(blockID flow.Identifier) ([]flow.TransactionResultErrorMessage, error) { + tx := t.db.NewTransaction(false) + defer tx.Discard() + key := KeyFromBlockID(blockID) + transactionResultErrorMessages, err := t.blockCache.Get(key)(tx) + if err != nil { + return nil, err + } + return transactionResultErrorMessages, nil +} diff --git a/storage/badger/transaction_result_error_messages_test.go b/storage/badger/transaction_result_error_messages_test.go new file mode 100644 index 00000000000..e21e8aaf348 --- /dev/null +++ b/storage/badger/transaction_result_error_messages_test.go @@ -0,0 +1,110 @@ +package badger_test + +import ( + "fmt" + "testing" + + "github.com/dgraph-io/badger/v2" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" + "golang.org/x/exp/rand" + + "github.com/onflow/flow-go/model/flow" + "github.com/onflow/flow-go/module/metrics" + "github.com/onflow/flow-go/storage" + "github.com/onflow/flow-go/utils/unittest" + + bstorage "github.com/onflow/flow-go/storage/badger" +) + +func TestStoringTransactionResultErrorMessages(t *testing.T) { + unittest.RunWithBadgerDB(t, func(db *badger.DB) { + metrics := metrics.NewNoopCollector() + store := bstorage.NewTransactionResultErrorMessages(metrics, db, 1000) + + blockID := unittest.IdentifierFixture() + + // test db Exists by block id + exists, err := store.Exists(blockID) + require.NoError(t, err) + require.False(t, exists) + + // check retrieving by ByBlockID + messages, err := store.ByBlockID(blockID) + require.NoError(t, err) + require.Nil(t, messages) + + txErrorMessages := make([]flow.TransactionResultErrorMessage, 0) + for i := 0; i < 10; i++ { + expected := flow.TransactionResultErrorMessage{ + TransactionID: unittest.IdentifierFixture(), + ErrorMessage: fmt.Sprintf("a runtime error %d", i), + ExecutorID: unittest.IdentifierFixture(), + Index: rand.Uint32(), + } + txErrorMessages = append(txErrorMessages, expected) + } + err = store.Store(blockID, txErrorMessages) + require.NoError(t, err) + + // test db Exists by block id + exists, err = store.Exists(blockID) + require.NoError(t, err) + require.True(t, exists) + + // check retrieving by ByBlockIDTransactionID + for _, txErrorMessage := range txErrorMessages { + actual, err := store.ByBlockIDTransactionID(blockID, txErrorMessage.TransactionID) + require.NoError(t, err) + assert.Equal(t, txErrorMessage, *actual) + } + + // check retrieving by ByBlockIDTransactionIndex + for _, txErrorMessage := range txErrorMessages { + actual, err := store.ByBlockIDTransactionIndex(blockID, txErrorMessage.Index) + require.NoError(t, err) + assert.Equal(t, txErrorMessage, *actual) + } + + // check retrieving by ByBlockID + actual, err := store.ByBlockID(blockID) + require.NoError(t, err) + assert.Equal(t, txErrorMessages, actual) + + // test loading from database + newStore := bstorage.NewTransactionResultErrorMessages(metrics, db, 1000) + for _, txErrorMessage := range txErrorMessages { + actual, err := newStore.ByBlockIDTransactionID(blockID, txErrorMessage.TransactionID) + require.NoError(t, err) + assert.Equal(t, txErrorMessage, *actual) + } + + // check retrieving by index from both cache and db + for i, txErrorMessage := range txErrorMessages { + actual, err := store.ByBlockIDTransactionIndex(blockID, txErrorMessage.Index) + require.NoError(t, err) + assert.Equal(t, txErrorMessages[i], *actual) + + actual, err = newStore.ByBlockIDTransactionIndex(blockID, txErrorMessage.Index) + require.NoError(t, err) + assert.Equal(t, txErrorMessages[i], *actual) + } + }) +} + +func TestReadingNotStoreTransactionResultErrorMessage(t *testing.T) { + unittest.RunWithBadgerDB(t, func(db *badger.DB) { + metrics := metrics.NewNoopCollector() + store := bstorage.NewTransactionResultErrorMessages(metrics, db, 1000) + + blockID := unittest.IdentifierFixture() + txID := unittest.IdentifierFixture() + txIndex := rand.Uint32() + + _, err := store.ByBlockIDTransactionID(blockID, txID) + assert.ErrorIs(t, err, storage.ErrNotFound) + + _, err = store.ByBlockIDTransactionIndex(blockID, txIndex) + assert.ErrorIs(t, err, storage.ErrNotFound) + }) +} diff --git a/storage/mock/transaction_result_error_messages.go b/storage/mock/transaction_result_error_messages.go new file mode 100644 index 00000000000..c1bebf7f326 --- /dev/null +++ b/storage/mock/transaction_result_error_messages.go @@ -0,0 +1,163 @@ +// Code generated by mockery v2.43.2. DO NOT EDIT. + +package mock + +import ( + flow "github.com/onflow/flow-go/model/flow" + mock "github.com/stretchr/testify/mock" +) + +// TransactionResultErrorMessages is an autogenerated mock type for the TransactionResultErrorMessages type +type TransactionResultErrorMessages struct { + mock.Mock +} + +// ByBlockID provides a mock function with given fields: id +func (_m *TransactionResultErrorMessages) ByBlockID(id flow.Identifier) ([]flow.TransactionResultErrorMessage, error) { + ret := _m.Called(id) + + if len(ret) == 0 { + panic("no return value specified for ByBlockID") + } + + var r0 []flow.TransactionResultErrorMessage + var r1 error + if rf, ok := ret.Get(0).(func(flow.Identifier) ([]flow.TransactionResultErrorMessage, error)); ok { + return rf(id) + } + if rf, ok := ret.Get(0).(func(flow.Identifier) []flow.TransactionResultErrorMessage); ok { + r0 = rf(id) + } else { + if ret.Get(0) != nil { + r0 = ret.Get(0).([]flow.TransactionResultErrorMessage) + } + } + + if rf, ok := ret.Get(1).(func(flow.Identifier) error); ok { + r1 = rf(id) + } else { + r1 = ret.Error(1) + } + + return r0, r1 +} + +// ByBlockIDTransactionID provides a mock function with given fields: blockID, transactionID +func (_m *TransactionResultErrorMessages) ByBlockIDTransactionID(blockID flow.Identifier, transactionID flow.Identifier) (*flow.TransactionResultErrorMessage, error) { + ret := _m.Called(blockID, transactionID) + + if len(ret) == 0 { + panic("no return value specified for ByBlockIDTransactionID") + } + + var r0 *flow.TransactionResultErrorMessage + var r1 error + if rf, ok := ret.Get(0).(func(flow.Identifier, flow.Identifier) (*flow.TransactionResultErrorMessage, error)); ok { + return rf(blockID, transactionID) + } + if rf, ok := ret.Get(0).(func(flow.Identifier, flow.Identifier) *flow.TransactionResultErrorMessage); ok { + r0 = rf(blockID, transactionID) + } else { + if ret.Get(0) != nil { + r0 = ret.Get(0).(*flow.TransactionResultErrorMessage) + } + } + + if rf, ok := ret.Get(1).(func(flow.Identifier, flow.Identifier) error); ok { + r1 = rf(blockID, transactionID) + } else { + r1 = ret.Error(1) + } + + return r0, r1 +} + +// ByBlockIDTransactionIndex provides a mock function with given fields: blockID, txIndex +func (_m *TransactionResultErrorMessages) ByBlockIDTransactionIndex(blockID flow.Identifier, txIndex uint32) (*flow.TransactionResultErrorMessage, error) { + ret := _m.Called(blockID, txIndex) + + if len(ret) == 0 { + panic("no return value specified for ByBlockIDTransactionIndex") + } + + var r0 *flow.TransactionResultErrorMessage + var r1 error + if rf, ok := ret.Get(0).(func(flow.Identifier, uint32) (*flow.TransactionResultErrorMessage, error)); ok { + return rf(blockID, txIndex) + } + if rf, ok := ret.Get(0).(func(flow.Identifier, uint32) *flow.TransactionResultErrorMessage); ok { + r0 = rf(blockID, txIndex) + } else { + if ret.Get(0) != nil { + r0 = ret.Get(0).(*flow.TransactionResultErrorMessage) + } + } + + if rf, ok := ret.Get(1).(func(flow.Identifier, uint32) error); ok { + r1 = rf(blockID, txIndex) + } else { + r1 = ret.Error(1) + } + + return r0, r1 +} + +// Exists provides a mock function with given fields: blockID +func (_m *TransactionResultErrorMessages) Exists(blockID flow.Identifier) (bool, error) { + ret := _m.Called(blockID) + + if len(ret) == 0 { + panic("no return value specified for Exists") + } + + var r0 bool + var r1 error + if rf, ok := ret.Get(0).(func(flow.Identifier) (bool, error)); ok { + return rf(blockID) + } + if rf, ok := ret.Get(0).(func(flow.Identifier) bool); ok { + r0 = rf(blockID) + } else { + r0 = ret.Get(0).(bool) + } + + if rf, ok := ret.Get(1).(func(flow.Identifier) error); ok { + r1 = rf(blockID) + } else { + r1 = ret.Error(1) + } + + return r0, r1 +} + +// Store provides a mock function with given fields: blockID, transactionResultErrorMessages +func (_m *TransactionResultErrorMessages) Store(blockID flow.Identifier, transactionResultErrorMessages []flow.TransactionResultErrorMessage) error { + ret := _m.Called(blockID, transactionResultErrorMessages) + + if len(ret) == 0 { + panic("no return value specified for Store") + } + + var r0 error + if rf, ok := ret.Get(0).(func(flow.Identifier, []flow.TransactionResultErrorMessage) error); ok { + r0 = rf(blockID, transactionResultErrorMessages) + } else { + r0 = ret.Error(0) + } + + return r0 +} + +// NewTransactionResultErrorMessages creates a new instance of TransactionResultErrorMessages. It also registers a testing interface on the mock and a cleanup function to assert the mocks expectations. +// The first argument is typically a *testing.T value. +func NewTransactionResultErrorMessages(t interface { + mock.TestingT + Cleanup(func()) +}) *TransactionResultErrorMessages { + mock := &TransactionResultErrorMessages{} + mock.Mock.Test(t) + + t.Cleanup(func() { mock.AssertExpectations(t) }) + + return mock +} diff --git a/storage/transaction_results.go b/storage/transaction_results.go index bb66023bc83..5267b8e268d 100644 --- a/storage/transaction_results.go +++ b/storage/transaction_results.go @@ -33,3 +33,35 @@ type LightTransactionResults interface { // ByBlockID gets all transaction results for a block, ordered by transaction index ByBlockID(id flow.Identifier) ([]flow.LightTransactionResult, error) } + +// TransactionResultErrorMessages represents persistent storage for transaction result error messages +type TransactionResultErrorMessages interface { + + // Store will store transaction result error messages for the given block ID. + // + // No errors are expected during normal operation. + Store(blockID flow.Identifier, transactionResultErrorMessages []flow.TransactionResultErrorMessage) error + + // Exists returns true if transaction result error messages for the given ID have been stored. + // + // No errors are expected during normal operation. + Exists(blockID flow.Identifier) (bool, error) + + // ByBlockIDTransactionID returns the transaction result error message for the given block ID and transaction ID. + // + // Expected errors during normal operation: + // - `storage.ErrNotFound` if no transaction error message is known at given block and transaction id. + ByBlockIDTransactionID(blockID flow.Identifier, transactionID flow.Identifier) (*flow.TransactionResultErrorMessage, error) + + // ByBlockIDTransactionIndex returns the transaction result error message for the given blockID and transaction index. + // + // Expected errors during normal operation: + // - `storage.ErrNotFound` if no transaction error message is known at given block and transaction index. + ByBlockIDTransactionIndex(blockID flow.Identifier, txIndex uint32) (*flow.TransactionResultErrorMessage, error) + + // ByBlockID gets all transaction result error messages for a block, ordered by transaction index. + // + // Expected errors during normal operation: + // - `storage.ErrNotFound` if no transaction error messages is known at given block. + ByBlockID(id flow.Identifier) ([]flow.TransactionResultErrorMessage, error) +} diff --git a/utils/unittest/mocks/closer.go b/utils/unittest/mocks/closer.go new file mode 100644 index 00000000000..21961f0f982 --- /dev/null +++ b/utils/unittest/mocks/closer.go @@ -0,0 +1,5 @@ +package mocks + +type MockCloser struct{} + +func (mc *MockCloser) Close() error { return nil }