Validate TransactionArgs passed in eth_estimateGas & eth_call JSON-RP…

…C endpoints

api/api.go

@@ -487,6 +487,11 @@ func (b *BlockChainAPI) Call(
 		return nil, err
 	}
 
+	err := args.Validate()
+	if err != nil {
+		return nil, err
+	}
+
 	evmHeight, err := b.getBlockNumber(blockNumberOrHash)
 	if err != nil {
 		return handleError[hexutil.Bytes](b.logger, err)
@@ -628,6 +633,11 @@ func (b *BlockChainAPI) EstimateGas(
 		return 0, err
 	}
 
+	err := args.Validate()
+	if err != nil {
+		return 0, err
+	}
+
 	tx, err := encodeTxFromArgs(args)
 	if err != nil {
 		return hexutil.Uint64(blockGasLimit), nil // return block gas limit

api/models.go

@@ -1,6 +1,11 @@
 package api
 
 import (
+	"bytes"
+	"errors"
+	"fmt"
+	"math/big"
+
 	errs "github.com/onflow/flow-evm-gateway/api/errors"
 	"github.com/onflow/flow-evm-gateway/config"
 	"github.com/onflow/flow-evm-gateway/models"
@@ -33,6 +38,75 @@ type TransactionArgs struct {
 	ChainID    *hexutil.Big      `json:"chainId,omitempty"`
 }
 
+func (txArgs TransactionArgs) Validate() error {
+	// Prevent accidental erroneous usage of both 'input' and 'data' (show stopper)
+	if txArgs.Data != nil && txArgs.Input != nil && !bytes.Equal(*txArgs.Data, *txArgs.Input) {
+		return errors.New(`ambiguous request: both "data" and "input" are set and are not identical`)
+	}
+
+	// Place data on 'data'
+	var data []byte
+	if txArgs.Input != nil {
+		txArgs.Data = txArgs.Input
+	}
+	if txArgs.Data != nil {
+		data = *txArgs.Data
+	}
+
+	txDataLen := len(data)
+
+	// Contract creation doesn't validate call data, handle first
+	if txArgs.To == nil {
+		// Contract creation should contain sufficient data to deploy a contract. A
+		// typical error is omitting sender due to some quirk in the javascript call
+		// e.g. https://github.com/onflow/go-ethereum/issues/16106.
+		if txDataLen == 0 {
+			// Prevent sending ether into black hole (show stopper)
+			if txArgs.Value.ToInt().Cmp(big.NewInt(0)) > 0 {
+				return errors.New("transaction will create a contract with value but empty code")
+			}
+
+			// No value submitted at least, critically Warn, but don't blow up
+			return errors.New("transaction will create a contract with empty code")
+		}
+
+		if txDataLen < 40 { // arbitrary heuristic limit
+			return fmt.Errorf(
+				"transaction will create a contract, but the payload is suspiciously small (%d bytes)",
+				txDataLen,
+			)
+		}
+	}
+
+	// Not a contract creation, validate as a plain transaction
+	if txArgs.To != nil {
+		to := common.NewMixedcaseAddress(*txArgs.To)
+		if !to.ValidChecksum() {
+			return errors.New("invalid checksum on recipient address")
+		}
+
+		if bytes.Equal(to.Address().Bytes(), common.Address{}.Bytes()) {
+			return errors.New("transaction recipient is the zero address")
+		}
+
+		// If the data is not empty, validate that it has the 4byte prefix and the rest divisible by 32 bytes
+		if txDataLen > 0 {
+			if txDataLen < 4 {
+				return errors.New("transaction data is not valid ABI (missing the 4 byte call prefix)")
+			}
+
+			if n := txDataLen - 4; n%32 != 0 {
+				return fmt.Errorf(
+					"transaction data is not valid ABI (length should be a multiple of 32 (was %d))",
+					n,
+				)
+			}
+		}
+	}
+
+	return nil
+}
+
 // AccessListResult returns an optional accesslist
 // It's the result of the `debug_createAccessList` RPC call.
 // It contains an error if the transaction itself failed.

api/models_test.go

@@ -0,0 +1,149 @@
+package api
+
+import (
+	"encoding/hex"
+	"math/big"
+	"testing"
+
+	"github.com/onflow/go-ethereum/common"
+	"github.com/onflow/go-ethereum/common/hexutil"
+	"github.com/stretchr/testify/assert"
+	"github.com/stretchr/testify/require"
+)
+
+func TestValidateTransaction(t *testing.T) {
+	validToAddress := common.HexToAddress("0x000000000000000000000000000000000000dEaD")
+	zeroToAddress := common.HexToAddress("0x0000000000000000000000000000000000000000")
+
+	data, err := hex.DecodeString("c6888fa1")
+	require.NoError(t, err)
+	smallContractPayload := hexutil.Bytes(data)
+
+	data, err = hex.DecodeString("c6888f")
+	require.NoError(t, err)
+	invalidTxData := hexutil.Bytes(data)
+
+	data, err = hex.DecodeString("c6888fa1000000000000000000000000000000000000000000000000000000000000ab")
+	require.NoError(t, err)
+	invalidTxDataLen := hexutil.Bytes(data)
+
+	gasLimit := uint64(125_000)
+
+	nonce := uint64(1)
+	tests := map[string]struct {
+		txArgs TransactionArgs
+		valid  bool
+		errMsg string
+	}{
+		"valid transaction": {
+			txArgs: TransactionArgs{
+				Nonce:    (*hexutil.Uint64)(&nonce),
+				To:       &validToAddress,
+				Value:    (*hexutil.Big)(big.NewInt(0)),
+				Gas:      (*hexutil.Uint64)(&gasLimit),
+				GasPrice: (*hexutil.Big)(big.NewInt(0)),
+				Data:     &hexutil.Bytes{},
+			},
+			valid:  true,
+			errMsg: "",
+		},
+		"conflicting input and data": {
+			txArgs: TransactionArgs{
+				Nonce:    (*hexutil.Uint64)(&nonce),
+				To:       &validToAddress,
+				Value:    (*hexutil.Big)(big.NewInt(0)),
+				Gas:      (*hexutil.Uint64)(&gasLimit),
+				GasPrice: (*hexutil.Big)(big.NewInt(0)),
+				Data:     &invalidTxDataLen,
+				Input:    &invalidTxData,
+			},
+			valid:  false,
+			errMsg: `ambiguous request: both "data" and "input" are set and are not identical`,
+		},
+		"send to 0 address": {
+			txArgs: TransactionArgs{
+				Nonce:    (*hexutil.Uint64)(&nonce),
+				To:       &zeroToAddress,
+				Value:    (*hexutil.Big)(big.NewInt(0)),
+				Gas:      (*hexutil.Uint64)(&gasLimit),
+				GasPrice: (*hexutil.Big)(big.NewInt(0)),
+				Data:     &hexutil.Bytes{},
+			},
+			valid:  false,
+			errMsg: "transaction recipient is the zero address",
+		},
+		"create empty contract (no value)": {
+			txArgs: TransactionArgs{
+				Nonce:    (*hexutil.Uint64)(&nonce),
+				To:       nil,
+				Value:    (*hexutil.Big)(big.NewInt(0)),
+				Gas:      (*hexutil.Uint64)(&gasLimit),
+				GasPrice: (*hexutil.Big)(big.NewInt(0)),
+				Data:     &hexutil.Bytes{},
+			},
+			valid:  false,
+			errMsg: "transaction will create a contract with empty code",
+		},
+		"create empty contract (with value)": {
+			txArgs: TransactionArgs{
+				Nonce:    (*hexutil.Uint64)(&nonce),
+				To:       nil,
+				Value:    (*hexutil.Big)(big.NewInt(150)),
+				Gas:      (*hexutil.Uint64)(&gasLimit),
+				GasPrice: (*hexutil.Big)(big.NewInt(0)),
+				Data:     &hexutil.Bytes{},
+			},
+			valid:  false,
+			errMsg: "transaction will create a contract with value but empty code",
+		},
+		"small payload for create": {
+			txArgs: TransactionArgs{
+				Nonce:    (*hexutil.Uint64)(&nonce),
+				To:       nil,
+				Value:    (*hexutil.Big)(big.NewInt(150)),
+				Gas:      (*hexutil.Uint64)(&gasLimit),
+				GasPrice: (*hexutil.Big)(big.NewInt(0)),
+				Data:     &smallContractPayload,
+			},
+			valid:  false,
+			errMsg: "transaction will create a contract, but the payload is suspiciously small (4 bytes)",
+		},
+		"tx data length < 4": {
+			txArgs: TransactionArgs{
+				Nonce:    (*hexutil.Uint64)(&nonce),
+				To:       &validToAddress,
+				Value:    (*hexutil.Big)(big.NewInt(150)),
+				Gas:      (*hexutil.Uint64)(&gasLimit),
+				GasPrice: (*hexutil.Big)(big.NewInt(0)),
+				Data:     &invalidTxData,
+			},
+			valid:  false,
+			errMsg: "transaction data is not valid ABI (missing the 4 byte call prefix)",
+		},
+		"tx data (excluding function selector) not divisible by 32": {
+			txArgs: TransactionArgs{
+				Nonce:    (*hexutil.Uint64)(&nonce),
+				To:       &validToAddress,
+				Value:    (*hexutil.Big)(big.NewInt(150)),
+				Gas:      (*hexutil.Uint64)(&gasLimit),
+				GasPrice: (*hexutil.Big)(big.NewInt(0)),
+				Data:     &invalidTxDataLen,
+			},
+			valid:  false,
+			errMsg: "transaction data is not valid ABI (length should be a multiple of 32 (was 31))",
+		},
+	}
+
+	for name, tc := range tests {
+		t.Run(name, func(t *testing.T) {
+			err := tc.txArgs.Validate()
+			if tc.valid {
+				require.NoError(t, err)
+			} else {
+				require.Error(t, err)
+				assert.ErrorContains(t, err, tc.errMsg)
+			}
+		})
+	}
+
+}