Merge pull request #4917 from onflow/petera/4916-en-get-account

[Execution] Return OutOfRange instead of Internal when account block is not cached

engine/execution/rpc/engine.go

@@ -26,6 +26,7 @@ import (
 	"github.com/onflow/flow-go/engine/common/rpc"
 	"github.com/onflow/flow-go/engine/common/rpc/convert"
 	exeEng "github.com/onflow/flow-go/engine/execution"
+	"github.com/onflow/flow-go/engine/execution/scripts"
 	fvmerrors "github.com/onflow/flow-go/fvm/errors"
 	"github.com/onflow/flow-go/model/flow"
 	"github.com/onflow/flow-go/state/protocol"
@@ -543,13 +544,16 @@ func (h *handler) GetAccountAtBlockID(
 	}
 
 	value, err := h.engine.GetAccount(ctx, flowAddress, blockFlowID)
-	if errors.Is(err, storage.ErrNotFound) {
-		return nil, status.Errorf(codes.NotFound, "account with address %s not found", flowAddress)
-	}
-	if fvmerrors.IsAccountNotFoundError(err) {
-		return nil, status.Errorf(codes.NotFound, "account not found")
-	}
 	if err != nil {
+		if errors.Is(err, scripts.ErrStateCommitmentPruned) {
+			return nil, status.Errorf(codes.OutOfRange, "state for block ID %s not available", blockFlowID)
+		}
+		if errors.Is(err, storage.ErrNotFound) {
+			return nil, status.Errorf(codes.NotFound, "account with address %s not found", flowAddress)
+		}
+		if fvmerrors.IsAccountNotFoundError(err) {
+			return nil, status.Errorf(codes.NotFound, "account not found")
+		}
 		return nil, status.Errorf(codes.Internal, "failed to get account: %v", err)
 	}
 

engine/execution/rpc/engine_test.go

@@ -3,6 +3,7 @@ package rpc
 import (
 	"context"
 	"errors"
+	"fmt"
 	"math/rand"
 	"testing"
 
@@ -19,6 +20,7 @@ import (
 
 	"github.com/onflow/flow-go/engine/common/rpc/convert"
 	mockEng "github.com/onflow/flow-go/engine/execution/mock"
+	"github.com/onflow/flow-go/engine/execution/scripts"
 	"github.com/onflow/flow-go/model/flow"
 	realstorage "github.com/onflow/flow-go/storage"
 	storage "github.com/onflow/flow-go/storage/mock"
@@ -324,6 +326,27 @@ func (suite *Suite) TestGetAccountAtBlockID() {
 		suite.Require().Error(err)
 		errors.Is(err, status.Error(codes.InvalidArgument, ""))
 	})
+
+	suite.Run("valid request for unavailable data", func() {
+		suite.commits.On("ByBlockID", id).Return(nil, nil).Once()
+
+		expectedErr := fmt.Errorf(
+			"failed to execute script at block (%s): %w (%s). "+
+				"this error usually happens if the reference "+
+				"block for this script is not set to a recent block.",
+			id,
+			scripts.ErrStateCommitmentPruned,
+			unittest.IdentifierFixture(),
+		)
+
+		mockEngine.On("GetAccount", mock.Anything, serviceAddress, id).Return(nil, expectedErr).Once()
+
+		req := createReq(id[:], serviceAddress.Bytes())
+
+		resp, err := handler.GetAccountAtBlockID(context.Background(), req)
+		suite.Assert().Nil(resp)
+		suite.Assert().Equal(codes.OutOfRange, status.Code(err))
+	})
 }
 
 // Test GetRegisterAtBlockID tests the GetRegisterAtBlockID API call

engine/execution/scripts/engine.go

@@ -15,6 +15,8 @@ import (
 	"github.com/onflow/flow-go/state/protocol"
 )
 
+var ErrStateCommitmentPruned = fmt.Errorf("state commitment not found")
+
 type Engine struct {
 	unit          *engine.Unit
 	log           zerolog.Logger
@@ -63,7 +65,14 @@ func (e *Engine) ExecuteScriptAtBlockID(
 	// return early if state with the given state commitment is not in memory
 	// and already purged. This reduces allocations for scripts targeting old blocks.
 	if !e.execState.HasState(stateCommit) {
-		return nil, fmt.Errorf("failed to execute script at block (%s): state commitment not found (%s). this error usually happens if the reference block for this script is not set to a recent block", blockID.String(), hex.EncodeToString(stateCommit[:]))
+		return nil, fmt.Errorf(
+			"failed to execute script at block (%s): %w (%s). "+
+				"this error usually happens if the reference "+
+				"block for this script is not set to a recent block.",
+			blockID.String(),
+			ErrStateCommitmentPruned,
+			hex.EncodeToString(stateCommit[:]),
+		)
 	}
 
 	header, err := e.state.AtBlockID(blockID).Head()
@@ -117,10 +126,11 @@ func (e *Engine) GetAccount(
 	// and already purged. This reduces allocations for get accounts targeting old blocks.
 	if !e.execState.HasState(stateCommit) {
 		return nil, fmt.Errorf(
-			"failed to get account at block (%s): state commitment not "+
-				"found (%s). this error usually happens if the reference "+
+			"failed to get account at block (%s): %w (%s). "+
+				"this error usually happens if the reference "+
 				"block for this script is not set to a recent block.",
 			blockID.String(),
+			ErrStateCommitmentPruned,
 			hex.EncodeToString(stateCommit[:]))
 	}