Skip to content

Commit

Permalink
Merge pull request #4920 from onflow/petera/4918-fix-getaccount-valid…
Browse files Browse the repository at this point in the history
…ation

[Access] Improve logging and validation in local script exec
  • Loading branch information
peterargue committed Nov 2, 2023
1 parent dad2b44 commit 52fcbf4
Show file tree
Hide file tree
Showing 2 changed files with 32 additions and 13 deletions.
29 changes: 16 additions & 13 deletions access/handler.go
Original file line number Diff line number Diff line change
Expand Up @@ -117,7 +117,7 @@ func (h *Handler) GetBlockHeaderByID(
) (*access.BlockHeaderResponse, error) {
id, err := convert.BlockID(req.GetId())
if err != nil {
return nil, err
return nil, status.Errorf(codes.InvalidArgument, "invalid block id: %v", err)
}
header, status, err := h.api.GetBlockHeaderByID(ctx, id)
if err != nil {
Expand Down Expand Up @@ -157,7 +157,7 @@ func (h *Handler) GetBlockByID(
) (*access.BlockResponse, error) {
id, err := convert.BlockID(req.GetId())
if err != nil {
return nil, err
return nil, status.Errorf(codes.InvalidArgument, "invalid block id: %v", err)
}
block, status, err := h.api.GetBlockByID(ctx, id)
if err != nil {
Expand All @@ -175,7 +175,7 @@ func (h *Handler) GetCollectionByID(

id, err := convert.CollectionID(req.GetId())
if err != nil {
return nil, err
return nil, status.Errorf(codes.InvalidArgument, "invalid collection id: %v", err)
}

col, err := h.api.GetCollectionByID(ctx, id)
Expand Down Expand Up @@ -230,7 +230,7 @@ func (h *Handler) GetTransaction(

id, err := convert.TransactionID(req.GetId())
if err != nil {
return nil, err
return nil, status.Errorf(codes.InvalidArgument, "invalid transaction id: %v", err)
}

tx, err := h.api.GetTransaction(ctx, id)
Expand All @@ -253,15 +253,15 @@ func (h *Handler) GetTransactionResult(

transactionID, err := convert.TransactionID(req.GetId())
if err != nil {
return nil, err
return nil, status.Errorf(codes.InvalidArgument, "invalid transaction id: %v", err)
}

blockId := flow.ZeroID
requestBlockId := req.GetBlockId()
if requestBlockId != nil {
blockId, err = convert.BlockID(requestBlockId)
if err != nil {
return nil, err
return nil, status.Errorf(codes.InvalidArgument, "invalid block id: %v", err)
}
}

Expand All @@ -270,7 +270,7 @@ func (h *Handler) GetTransactionResult(
if requestCollectionId != nil {
collectionId, err = convert.CollectionID(requestCollectionId)
if err != nil {
return nil, err
return nil, status.Errorf(codes.InvalidArgument, "invalid collection id: %v", err)
}
}

Expand All @@ -294,7 +294,7 @@ func (h *Handler) GetTransactionResultsByBlockID(

id, err := convert.BlockID(req.GetBlockId())
if err != nil {
return nil, err
return nil, status.Errorf(codes.InvalidArgument, "invalid block id: %v", err)
}

eventEncodingVersion := req.GetEventEncodingVersion()
Expand All @@ -318,7 +318,7 @@ func (h *Handler) GetTransactionsByBlockID(

id, err := convert.BlockID(req.GetBlockId())
if err != nil {
return nil, err
return nil, status.Errorf(codes.InvalidArgument, "invalid block id: %v", err)
}

transactions, err := h.api.GetTransactionsByBlockID(ctx, id)
Expand All @@ -342,7 +342,7 @@ func (h *Handler) GetTransactionResultByIndex(

blockID, err := convert.BlockID(req.GetBlockId())
if err != nil {
return nil, err
return nil, status.Errorf(codes.InvalidArgument, "invalid block id: %v", err)
}

eventEncodingVersion := req.GetEventEncodingVersion()
Expand All @@ -365,7 +365,10 @@ func (h *Handler) GetAccount(
) (*access.GetAccountResponse, error) {
metadata := h.buildMetadataResponse()

address := flow.BytesToAddress(req.GetAddress())
address, err := convert.Address(req.GetAddress(), h.chain)
if err != nil {
return nil, status.Errorf(codes.InvalidArgument, "invalid address: %v", err)
}

account, err := h.api.GetAccount(ctx, address)
if err != nil {
Expand All @@ -392,7 +395,7 @@ func (h *Handler) GetAccountAtLatestBlock(

address, err := convert.Address(req.GetAddress(), h.chain)
if err != nil {
return nil, err
return nil, status.Errorf(codes.InvalidArgument, "invalid address: %v", err)
}

account, err := h.api.GetAccountAtLatestBlock(ctx, address)
Expand All @@ -419,7 +422,7 @@ func (h *Handler) GetAccountAtBlockHeight(

address, err := convert.Address(req.GetAddress(), h.chain)
if err != nil {
return nil, err
return nil, status.Errorf(codes.InvalidArgument, "invalid address: %v", err)
}

account, err := h.api.GetAccountAtBlockHeight(ctx, address, req.GetBlockHeight())
Expand Down
16 changes: 16 additions & 0 deletions engine/access/rpc/backend/backend_scripts.go
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ import (
"context"
"crypto/md5" //nolint:gosec
"errors"
"strings"
"time"

lru "github.com/hashicorp/golang-lru/v2"
Expand Down Expand Up @@ -304,6 +305,21 @@ func (b *backendScripts) compareScriptExecutionResults(
return
}

// error strings generally won't match since the code paths are slightly different
// check if the underlying error is the same
if status.Code(execErr) == status.Code(localErr) {
// both script execution implementations use the same engine, which adds
// "failed to execute script at block" to the message before returning. Any characters
// before this can be ignored. The string that comes after is the original error and
// should match.
execParts := strings.Split(execErr.Error(), "failed to execute script at block")
localParts := strings.Split(localErr.Error(), "failed to execute script at block")
if len(execParts) == 2 && len(localParts) == 2 && execParts[1] == localParts[1] {
b.metrics.ScriptExecutionErrorMatch()
return
}
}

b.metrics.ScriptExecutionErrorMismatch()
b.logScriptExecutionComparison(execNodeResult, execErr, localResult, localErr, blockID, script, insecureScriptHash,
"cadence errors from local execution do not match and EN")
Expand Down

0 comments on commit 52fcbf4

Please sign in to comment.