Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Bugfix invalid block number or hash #313

Merged
merged 5 commits into from
Jun 25, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
39 changes: 25 additions & 14 deletions api/api.go
Original file line number Diff line number Diff line change
Expand Up @@ -494,8 +494,7 @@ func (b *BlockChainAPI) Call(

tx, err := encodeTxFromArgs(args)
if err != nil {
b.logger.Error().Err(err).Msg("failed to encode transaction for call")
return handleError[hexutil.Bytes](b.logger, errs.ErrInternal)
return handleError[hexutil.Bytes](b.logger, err)
}

// Default address in case user does not provide one
Expand Down Expand Up @@ -549,7 +548,7 @@ func (b *BlockChainAPI) GetLogs(

l, err := b.blocks.LatestEVMHeight()
if err != nil {
return nil, err
return handleError[[]*types.Log](b.logger, err)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Refactor repeated error handling in GetLogs.

The handleError function is used multiple times in the same method. Consider refactoring to reduce redundancy and improve code clarity.

func (b *BlockChainAPI) GetLogs(ctx context.Context, criteria filters.FilterCriteria) ([]*types.Log, error) {
	if err := rateLimit(ctx, b.limiter, b.logger); err != nil {
		return nil, err
	}

	// Existing logic...

	latest, err := b.blocks.LatestEVMHeight()
	if err != nil {
		return handleError[[]*types.Log](b.logger, err)
	}
	latestInt := big.NewInt(int64(latest))

	// Adjusted logic to use `handleError` once at the end.
	res, err := logs.NewRangeFilter(*from, *to, filter, b.receipts).Match()
	return handleError[[]*types.Log](b.logger, err)
}

Also applies to: 565-565, 570-570

}
latest := big.NewInt(int64(l))

Expand All @@ -563,10 +562,15 @@ func (b *BlockChainAPI) GetLogs(

f, err := logs.NewRangeFilter(*from, *to, filter, b.receipts)
if err != nil {
return nil, err
return handleError[[]*types.Log](b.logger, err)
}

res, err := f.Match()
if err != nil {
return handleError[[]*types.Log](b.logger, err)
}

return f.Match()
return res, nil
}

// GetTransactionCount returns the number of transactions the given address
Expand Down Expand Up @@ -626,7 +630,6 @@ func (b *BlockChainAPI) EstimateGas(

tx, err := encodeTxFromArgs(args)
if err != nil {
b.logger.Error().Err(err).Msg("failed to encode transaction for gas estimate")
return hexutil.Uint64(blockGasLimit), nil // return block gas limit
}

Expand Down Expand Up @@ -676,16 +679,20 @@ func (b *BlockChainAPI) GetCode(
// empty type.
func handleError[T any](log zerolog.Logger, err error) (T, error) {
var zero T
if errors.Is(err, storageErrs.ErrNotFound) {
// as per specification returning nil and nil for not found resources
switch {
// as per specification returning nil and nil for not found resources
case errors.Is(err, storageErrs.ErrNotFound):
return zero, nil
}
if errors.Is(err, requester.ErrOutOfRange) {
case errors.Is(err, storageErrs.ErrInvalidRange):
return zero, err
case errors.Is(err, requester.ErrOutOfRange):
return zero, fmt.Errorf("requested height is out of supported range")
case errors.Is(err, errs.ErrInvalid):
return zero, err
default:
log.Error().Err(err).Msg("api error")
return zero, errs.ErrInternal
}

log.Error().Err(err).Msg("api error")
return zero, errs.ErrInternal
}

func (b *BlockChainAPI) fetchBlockTransactions(
Expand Down Expand Up @@ -776,6 +783,10 @@ func (b *BlockChainAPI) prepareBlockResponse(
}

func (b *BlockChainAPI) getBlockNumber(blockNumberOrHash *rpc.BlockNumberOrHash) (int64, error) {
err := errors.Join(errs.ErrInvalid, fmt.Errorf("neither block number nor hash specified"))
if blockNumberOrHash == nil {
return 0, err
}
if number, ok := blockNumberOrHash.Number(); ok {
return number.Int64(), nil
}
Expand All @@ -789,7 +800,7 @@ func (b *BlockChainAPI) getBlockNumber(blockNumberOrHash *rpc.BlockNumberOrHash)
return int64(evmHeight), nil
}

return 0, fmt.Errorf("invalid arguments; neither block nor hash specified")
return 0, err
}

// FeeHistory returns transaction base fee per gas and effective priority fee
Expand Down
10 changes: 9 additions & 1 deletion api/encode_transaction.go
Original file line number Diff line number Diff line change
@@ -1,9 +1,12 @@
package api

import (
"errors"
"math/big"

"github.com/onflow/go-ethereum/core/types"

errs "github.com/onflow/flow-evm-gateway/api/errors"
)

const blockGasLimit uint64 = 15_000_000
Expand Down Expand Up @@ -44,5 +47,10 @@ func encodeTxFromArgs(args TransactionArgs) ([]byte, error) {
},
)

return tx.MarshalBinary()
enc, err := tx.MarshalBinary()
if err != nil {
return nil, errors.Join(err, errs.ErrInvalid)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Correct error wrapping syntax.

The errors.Join function does not exist in the standard Go errors package. Use fmt.Errorf for wrapping errors to maintain consistency and compatibility.

- return nil, errors.Join(err, errs.ErrInvalid)
+ return nil, fmt.Errorf("%w: %v", errs.ErrInvalid, err)
Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
return nil, errors.Join(err, errs.ErrInvalid)
return nil, fmt.Errorf("%w: %v", errs.ErrInvalid, err)

}

return enc, nil
}
Loading