Merge pull request #6554 from The-K-R-O-K/UlyanaAndrukhiv/4904-improv…

…e-error-messages-for-old-blocks

[Access] Improve error messages when querying old blocks

engine/access/rpc/backend/backend.go

@@ -3,6 +3,7 @@ package backend
 import (
 	"context"
 	"crypto/md5" //nolint:gosec
+	"errors"
 	"fmt"
 	"time"
 
@@ -641,3 +642,44 @@ func chooseFromPreferredENIDs(allENs flow.IdentityList, executorIDs flow.Identif
 
 	return chosenIDs
 }
+
+// resolveHeightError processes errors returned during height-based queries.
+// If the error is due to a block not being found, this function determines whether the queried
+// height falls outside the node's accessible range and provides context-sensitive error messages
+// based on spork and node root block heights.
+//
+// Parameters:
+// - stateParams: Protocol parameters that contain spork root and node root block heights.
+// - height: The queried block height.
+// - genericErr: The initial error returned when the block is not found.
+//
+// Expected errors during normal operation:
+// - storage.ErrNotFound - Indicates that the queried block does not exist in the local database.
+func resolveHeightError(
+	stateParams protocol.Params,
+	height uint64,
+	genericErr error,
+) error {
+	if !errors.Is(genericErr, storage.ErrNotFound) {
+		return genericErr
+	}
+
+	sporkRootBlockHeight := stateParams.SporkRootBlockHeight()
+	nodeRootBlockHeader := stateParams.SealedRoot().Height
+
+	if height < sporkRootBlockHeight {
+		return fmt.Errorf("block height %d is less than the spork root block height %d. Try to use a historic node: %w",
+			height,
+			sporkRootBlockHeight,
+			genericErr,
+		)
+	} else if height < nodeRootBlockHeader {
+		return fmt.Errorf("block height %d is less than the node's root block height %d. Try to use a different Access node: %w",
+			height,
+			nodeRootBlockHeader,
+			genericErr,
+		)
+	} else {
+		return genericErr
+	}
+}

engine/access/rpc/backend/backend_accounts.go

@@ -68,7 +68,7 @@ func (b *backendAccounts) GetAccountAtBlockHeight(
 ) (*flow.Account, error) {
 	blockID, err := b.headers.BlockIDByHeight(height)
 	if err != nil {
-		return nil, rpc.ConvertStorageError(err)
+		return nil, rpc.ConvertStorageError(resolveHeightError(b.state.Params(), height, err))
 	}
 
 	account, err := b.getAccountAtBlock(ctx, address, blockID, height)
@@ -108,7 +108,7 @@ func (b *backendAccounts) GetAccountBalanceAtBlockHeight(
 ) (uint64, error) {
 	blockID, err := b.headers.BlockIDByHeight(height)
 	if err != nil {
-		return 0, rpc.ConvertStorageError(err)
+		return 0, rpc.ConvertStorageError(resolveHeightError(b.state.Params(), height, err))
 	}
 
 	balance, err := b.getAccountBalanceAtBlock(ctx, address, blockID, height)
@@ -176,7 +176,7 @@ func (b *backendAccounts) GetAccountKeyAtBlockHeight(
 ) (*flow.AccountPublicKey, error) {
 	blockID, err := b.headers.BlockIDByHeight(height)
 	if err != nil {
-		return nil, rpc.ConvertStorageError(err)
+		return nil, rpc.ConvertStorageError(resolveHeightError(b.state.Params(), height, err))
 	}
 
 	accountKey, err := b.getAccountKeyAtBlock(ctx, address, keyIndex, blockID, height)
@@ -196,7 +196,7 @@ func (b *backendAccounts) GetAccountKeysAtBlockHeight(
 ) ([]flow.AccountPublicKey, error) {
 	blockID, err := b.headers.BlockIDByHeight(height)
 	if err != nil {
-		return nil, rpc.ConvertStorageError(err)
+		return nil, rpc.ConvertStorageError(resolveHeightError(b.state.Params(), height, err))
 	}
 
 	accountKeys, err := b.getAccountKeysAtBlock(ctx, address, blockID, height)
@@ -400,7 +400,7 @@ func (b *backendAccounts) getAccountFromLocalStorage(
 	// make sure data is available for the requested block
 	account, err := b.scriptExecutor.GetAccountAtBlockHeight(ctx, address, height)
 	if err != nil {
-		return nil, convertAccountError(err, address, height)
+		return nil, convertAccountError(resolveHeightError(b.state.Params(), height, err), address, height)
 	}
 	return account, nil
 }

engine/access/rpc/backend/backend_accounts_test.go

@@ -238,6 +238,8 @@ func (s *BackendAccountsSuite) TestGetAccountFromStorage_Fails() {
 		scriptExecutor.On("GetAccountAtBlockHeight", mock.Anything, s.failingAddress, s.block.Header.Height).
 			Return(nil, tt.err).Times(3)
 
+		s.state.On("Params").Return(s.params).Times(3)
+
 		s.Run(fmt.Sprintf("GetAccount - fails with %v", tt.err), func() {
 			s.testGetAccount(ctx, backend, tt.statusCode)
 		})
@@ -247,6 +249,9 @@ func (s *BackendAccountsSuite) TestGetAccountFromStorage_Fails() {
 		})
 
 		s.Run(fmt.Sprintf("GetAccountAtBlockHeight - fails with %v", tt.err), func() {
+			s.params.On("SporkRootBlockHeight").Return(s.block.Header.Height-10, nil)
+			s.params.On("SealedRoot").Return(s.block.Header, nil)
+
 			s.testGetAccountAtBlockHeight(ctx, backend, tt.statusCode)
 		})
 	}
@@ -279,6 +284,9 @@ func (s *BackendAccountsSuite) TestGetAccountFromFailover_HappyPath() {
 		})
 
 		s.Run(fmt.Sprintf("GetAccountAtBlockHeight - happy path - recovers %v", errToReturn), func() {
+			s.params.On("SporkRootBlockHeight").Return(s.block.Header.Height-10, nil)
+			s.params.On("SealedRoot").Return(s.block.Header, nil)
+
 			s.testGetAccountAtBlockHeight(ctx, backend, codes.OK)
 		})
 	}

engine/access/rpc/backend/backend_block_details.go

@@ -80,7 +80,7 @@ func (b *backendBlockDetails) GetBlockByID(ctx context.Context, id flow.Identifi
 func (b *backendBlockDetails) GetBlockByHeight(ctx context.Context, height uint64) (*flow.Block, flow.BlockStatus, error) {
 	block, err := b.blocks.ByHeight(height)
 	if err != nil {
-		return nil, flow.BlockStatusUnknown, rpc.ConvertStorageError(err)
+		return nil, flow.BlockStatusUnknown, rpc.ConvertStorageError(resolveHeightError(b.state.Params(), height, err))
 	}
 
 	stat, err := b.getBlockStatus(ctx, block)

engine/access/rpc/backend/backend_block_headers.go

@@ -69,7 +69,7 @@ func (b *backendBlockHeaders) GetBlockHeaderByID(ctx context.Context, id flow.Id
 func (b *backendBlockHeaders) GetBlockHeaderByHeight(ctx context.Context, height uint64) (*flow.Header, flow.BlockStatus, error) {
 	header, err := b.headers.ByHeight(height)
 	if err != nil {
-		return nil, flow.BlockStatusUnknown, rpc.ConvertStorageError(err)
+		return nil, flow.BlockStatusUnknown, rpc.ConvertStorageError(resolveHeightError(b.state.Params(), height, err))
 	}
 
 	stat, err := b.getBlockStatus(ctx, header)

engine/access/rpc/backend/backend_events.go

@@ -104,7 +104,7 @@ func (b *backendEvents) GetEventsForHeightRange(
 		// and avoids calculating header.ID() for each block.
 		blockID, err := b.headers.BlockIDByHeight(i)
 		if err != nil {
-			return nil, rpc.ConvertStorageError(fmt.Errorf("failed to get blockID for %d: %w", i, err))
+			return nil, rpc.ConvertStorageError(resolveHeightError(b.state.Params(), i, err))
 		}
 		header, err := b.headers.ByBlockID(blockID)
 		if err != nil {

engine/access/rpc/backend/backend_events_test.go

@@ -431,6 +431,39 @@ func (s *BackendEventsSuite) TestGetEventsForHeightRange_HandlesErrors() {
 		s.Assert().Equal(codes.OutOfRange, status.Code(err))
 		s.Assert().Nil(response)
 	})
+
+	s.state.On("Params").Return(s.params)
+
+	s.Run("returns error for startHeight < spork root height", func() {
+		backend := s.defaultBackend()
+
+		sporkRootHeight := s.blocks[0].Header.Height - 10
+		startHeight := sporkRootHeight - 1
+
+		s.params.On("SporkRootBlockHeight").Return(sporkRootHeight).Once()
+		s.params.On("SealedRoot").Return(s.rootHeader, nil).Once()
+
+		response, err := backend.GetEventsForHeightRange(ctx, targetEvent, startHeight, endHeight, encoding)
+		s.Assert().Equal(codes.NotFound, status.Code(err))
+		s.Assert().ErrorContains(err, "Try to use a historic node")
+		s.Assert().Nil(response)
+	})
+
+	s.Run("returns error for startHeight < node root height", func() {
+		backend := s.defaultBackend()
+
+		sporkRootHeight := s.blocks[0].Header.Height - 10
+		nodeRootHeader := unittest.BlockHeaderWithHeight(s.blocks[0].Header.Height)
+		startHeight := nodeRootHeader.Height - 5
+
+		s.params.On("SporkRootBlockHeight").Return(sporkRootHeight).Once()
+		s.params.On("SealedRoot").Return(nodeRootHeader, nil).Once()
+
+		response, err := backend.GetEventsForHeightRange(ctx, targetEvent, startHeight, endHeight, encoding)
+		s.Assert().Equal(codes.NotFound, status.Code(err))
+		s.Assert().ErrorContains(err, "Try to use a different Access node")
+		s.Assert().Nil(response)
+	})
 }
 
 func (s *BackendEventsSuite) TestGetEventsForBlockIDs_HandlesErrors() {

engine/access/rpc/backend/backend_scripts.go

@@ -104,7 +104,7 @@ func (b *backendScripts) ExecuteScriptAtBlockHeight(
 ) ([]byte, error) {
 	header, err := b.headers.ByHeight(blockHeight)
 	if err != nil {
-		return nil, rpc.ConvertStorageError(err)
+		return nil, rpc.ConvertStorageError(resolveHeightError(b.state.Params(), blockHeight, err))
 	}
 
 	return b.executeScript(ctx, newScriptExecutionRequest(header.ID(), blockHeight, script, arguments))

engine/access/rpc/backend/backend_test.go

@@ -17,7 +17,6 @@ import (
 	execproto "github.com/onflow/flow/protobuf/go/flow/execution"
 	"github.com/rs/zerolog"
 	"github.com/sony/gobreaker"
-	"github.com/stretchr/testify/assert"
 	"github.com/stretchr/testify/mock"
 	"github.com/stretchr/testify/require"
 	"github.com/stretchr/testify/suite"
@@ -1493,7 +1492,7 @@ func (suite *Suite) TestGetExecutionResultByID() {
 		// execute request
 		_, err = backend.GetExecutionResultByID(ctx, nonexistingID)
 
-		assert.Error(suite.T(), err)
+		suite.Assert().Error(err)
 	})
 
 	suite.Run("existing execution result id", func() {
@@ -1555,7 +1554,7 @@ func (suite *Suite) TestGetExecutionResultByBlockID() {
 		// execute request
 		_, err = backend.GetExecutionResultForBlockID(ctx, nonexistingBlockID)
 
-		assert.Error(suite.T(), err)
+		suite.Assert().Error(err)
 	})
 
 	suite.Run("existing execution results", func() {
@@ -2241,3 +2240,80 @@ func (suite *Suite) defaultBackendParams() Params {
 		VersionControl:           suite.versionControl,
 	}
 }
+
+// TestResolveHeightError tests the resolveHeightError function for various scenarios where the block height
+// is below the spork root height, below the node root height, above the node root height, or when a different
+// error is provided. It validates that resolveHeightError returns an appropriate error message for each case.
+//
+// Test cases:
+// 1) If height is below the spork root height, it suggests using a historic node.
+// 2) If height is below the node root height, it suggests using a different Access node.
+// 3) If height is above the node root height, it returns the original error without modification.
+// 4) If a non-storage-related error is provided, it returns the error as is.
+func (suite *Suite) TestResolveHeightError() {
+	tests := []struct {
+		name              string
+		height            uint64
+		sporkRootHeight   uint64
+		nodeRootHeight    uint64
+		genericErr        error
+		expectedErrorMsg  string
+		expectOriginalErr bool
+	}{
+		{
+			name:             "height below spork root height",
+			height:           uint64(50),
+			sporkRootHeight:  uint64(100),
+			nodeRootHeight:   uint64(200),
+			genericErr:       storage.ErrNotFound,
+			expectedErrorMsg: "block height %d is less than the spork root block height 100. Try to use a historic node: %v"},
+		{
+			name:              "height below node root height",
+			height:            uint64(150),
+			sporkRootHeight:   uint64(100),
+			nodeRootHeight:    uint64(200),
+			genericErr:        storage.ErrNotFound,
+			expectedErrorMsg:  "block height %d is less than the node's root block height 200. Try to use a different Access node: %v",
+			expectOriginalErr: false,
+		},
+		{
+			name:              "height above node root height",
+			height:            uint64(205),
+			sporkRootHeight:   uint64(100),
+			nodeRootHeight:    uint64(200),
+			genericErr:        storage.ErrNotFound,
+			expectedErrorMsg:  "%v",
+			expectOriginalErr: true,
+		},
+		{
+			name:              "non-storage related error",
+			height:            uint64(150),
+			sporkRootHeight:   uint64(100),
+			nodeRootHeight:    uint64(200),
+			genericErr:        fmt.Errorf("some other error"),
+			expectedErrorMsg:  "%v",
+			expectOriginalErr: true,
+		},
+	}
+
+	for _, test := range tests {
+		suite.T().Run(test.name, func(t *testing.T) {
+			stateParams := protocol.NewParams(suite.T())
+
+			if errors.Is(test.genericErr, storage.ErrNotFound) {
+				stateParams.On("SporkRootBlockHeight").Return(test.sporkRootHeight).Once()
+				sealedRootHeader := unittest.BlockHeaderWithHeight(test.nodeRootHeight)
+				stateParams.On("SealedRoot").Return(sealedRootHeader, nil).Once()
+			}
+
+			err := resolveHeightError(stateParams, test.height, test.genericErr)
+
+			if test.expectOriginalErr {
+				suite.Assert().True(errors.Is(err, test.genericErr))
+			} else {
+				expectedError := fmt.Sprintf(test.expectedErrorMsg, test.height, test.genericErr)
+				suite.Assert().Equal(err.Error(), expectedError)
+			}
+		})
+	}
+}