Skip to content

Commit

Permalink
Merge pull request #364 from onflow/relax-validation-checks
Browse files Browse the repository at this point in the history
Relax validation checks for transaction arguments/payload
  • Loading branch information
sideninja authored Jul 18, 2024
2 parents aec1191 + df2d1ef commit 4a7c7e2
Show file tree
Hide file tree
Showing 4 changed files with 1 addition and 143 deletions.
25 changes: 0 additions & 25 deletions api/models.go
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,6 @@ package api
import (
"bytes"
"errors"
"fmt"
"math/big"

errs "github.com/onflow/flow-evm-gateway/api/errors"
Expand Down Expand Up @@ -69,39 +68,15 @@ func (txArgs TransactionArgs) Validate() error {
// 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
Expand Down
42 changes: 1 addition & 41 deletions api/models_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -15,11 +15,7 @@ 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")
data, err := hex.DecodeString("c6888f")
require.NoError(t, err)
invalidTxData := hexutil.Bytes(data)

Expand Down Expand Up @@ -96,42 +92,6 @@ func TestValidateTransaction(t *testing.T) {
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 {
Expand Down
26 changes: 0 additions & 26 deletions models/transaction.go
Original file line number Diff line number Diff line change
Expand Up @@ -311,39 +311,13 @@ func ValidateTransaction(
// 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 tx.To() != nil {
to := common.NewMixedcaseAddress(*tx.To())
if !to.ValidChecksum() {
return errors.New("invalid checksum on recipient address")
}

if bytes.Equal(tx.To().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,
)
}
}
}

if err := txpool.ValidateTransaction(tx, head, signer, opts); err != nil {
Expand Down
51 changes: 0 additions & 51 deletions models/transaction_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -345,15 +345,6 @@ func TestValidateTransaction(t *testing.T) {
validToAddress := gethCommon.HexToAddress("0x000000000000000000000000000000000000dEaD")
zeroToAddress := gethCommon.HexToAddress("0x0000000000000000000000000000000000000000")

smallContractPayload, err := hex.DecodeString("c6888fa1")
require.NoError(t, err)

invalidTxData, err := hex.DecodeString("c6888f")
require.NoError(t, err)

invalidTxDataLen, err := hex.DecodeString("c6888fa1000000000000000000000000000000000000000000000000000000000000ab")
require.NoError(t, err)

tests := map[string]struct {
tx *gethTypes.Transaction
valid bool
Expand Down Expand Up @@ -415,48 +406,6 @@ func TestValidateTransaction(t *testing.T) {
valid: false,
errMsg: "transaction will create a contract with value but empty code",
},
"small payload for create": {
tx: gethTypes.NewTx(
&gethTypes.LegacyTx{
Nonce: 1,
To: nil,
Value: big.NewInt(150),
Gas: 53_000,
GasPrice: 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": {
tx: gethTypes.NewTx(
&gethTypes.LegacyTx{
Nonce: 1,
To: &validToAddress,
Value: big.NewInt(150),
Gas: 153_000,
GasPrice: 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": {
tx: gethTypes.NewTx(
&gethTypes.LegacyTx{
Nonce: 1,
To: &validToAddress,
Value: big.NewInt(150),
Gas: 153_000,
GasPrice: big.NewInt(0),
Data: invalidTxDataLen,
},
),
valid: false,
errMsg: "transaction data is not valid ABI (length should be a multiple of 32 (was 31))",
},
}

head := &gethTypes.Header{
Expand Down

0 comments on commit 4a7c7e2

Please sign in to comment.