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

Consolidate the validations for max TX gas limit #759

Merged
merged 2 commits into from
Feb 21, 2025
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
7 changes: 4 additions & 3 deletions api/api.go
Original file line number Diff line number Diff line change
Expand Up @@ -576,7 +576,7 @@ func (b *BlockChainAPI) Call(

res, err := b.evm.Call(tx, from, height, stateOverrides, blockOverrides)
if err != nil {
return handleError[hexutil.Bytes](err, l, b.collector)
return nil, err
}

return res, nil
Expand Down Expand Up @@ -725,7 +725,8 @@ func (b *BlockChainAPI) EstimateGas(

tx, err := encodeTxFromArgs(args)
if err != nil {
return hexutil.Uint64(BlockGasLimit), nil // return block gas limit
// return max tx gas limit
return hexutil.Uint64(models.TxMaxGasLimit), nil
}

// Default address in case user does not provide one
Expand All @@ -745,7 +746,7 @@ func (b *BlockChainAPI) EstimateGas(

estimatedGas, err := b.evm.EstimateGas(tx, from, height, stateOverrides)
if err != nil {
return handleError[hexutil.Uint64](err, l, b.collector)
return 0, err
}

return hexutil.Uint64(estimatedGas), nil
Expand Down
2 changes: 1 addition & 1 deletion api/debug.go
Original file line number Diff line number Diff line change
Expand Up @@ -194,7 +194,7 @@ func (d *DebugAPI) TraceCall(
flowEVM.StorageAccountAddress(d.config.FlowNetworkID),
d.registerStore,
blocksProvider,
BlockGasLimit,
models.TxMaxGasLimit,
)

view, err := viewProvider.GetBlockView(block.Height)
Expand Down
3 changes: 2 additions & 1 deletion api/utils.go
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ import (

ethTypes "github.com/onflow/flow-evm-gateway/eth/types"
"github.com/onflow/flow-evm-gateway/metrics"
"github.com/onflow/flow-evm-gateway/models"
errs "github.com/onflow/flow-evm-gateway/models/errors"
"github.com/onflow/flow-evm-gateway/storage"
"github.com/onflow/go-ethereum/common"
Expand Down Expand Up @@ -153,7 +154,7 @@ func encodeTxFromArgs(args ethTypes.TransactionArgs) (*types.DynamicFeeTx, error

// provide a high enough gas for the tx to be able to execute,
// capped by the gas set in transaction args.
gasLimit := BlockGasLimit
gasLimit := models.TxMaxGasLimit
if args.Gas != nil {
gasLimit = uint64(*args.Gas)
}
Expand Down
10 changes: 10 additions & 0 deletions models/transaction.go
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ import (
"math/big"

"github.com/onflow/cadence"
errs "github.com/onflow/flow-evm-gateway/models/errors"
"github.com/onflow/flow-go/fvm/evm/events"
"github.com/onflow/flow-go/fvm/evm/types"
"github.com/onflow/go-ethereum/common"
Expand All @@ -27,6 +28,11 @@ const (
// more expensive to propagate; larger transactions also take more resources
// to validate whether they fit into the pool or not.
TxMaxSize = 4 * TxSlotSize // 128KB

// TxMaxSize is the maximum valid gas limit for EVM transactions that are
// wrapped within a Cadence transaction, configured with 9999 as the
// computation limit.
TxMaxGasLimit = uint64(50_000_000)
)

var BaseFeePerGas = big.NewInt(1)
Expand Down Expand Up @@ -270,6 +276,10 @@ func ValidateTransaction(
signer gethTypes.Signer,
opts *txpool.ValidationOptions,
) error {
if tx.Gas() > TxMaxGasLimit {
return errs.NewTxGasLimitTooHighError(TxMaxGasLimit)
}

txDataLen := len(tx.Data())

// Contract creation doesn't validate call data, handle first
Expand Down
14 changes: 14 additions & 0 deletions models/transaction_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -303,6 +303,20 @@ func TestValidateTransaction(t *testing.T) {
valid: true,
errMsg: "",
},
"gas limit exceeds max allowed value": {
tx: gethTypes.NewTx(
&gethTypes.LegacyTx{
Nonce: 1,
To: &validToAddress,
Value: big.NewInt(0),
Gas: TxMaxGasLimit + 25_000,
GasPrice: big.NewInt(0),
Data: []byte{},
},
),
valid: false,
errMsg: "invalid: failed transaction: tx gas limit exceeds the max value of 50000000",
},
"send to 0 address": {
tx: gethTypes.NewTx(
&gethTypes.LegacyTx{
Expand Down
9 changes: 2 additions & 7 deletions services/requester/requester.go
Original file line number Diff line number Diff line change
Expand Up @@ -44,7 +44,6 @@ var (

const minFlowBalance = 2
const blockGasLimit = 120_000_000
const txMaxGasLimit = 50_000_000

// estimateGasErrorRatio is the amount of overestimation eth_estimateGas
// is allowed to produce in order to speed up calculations.
Expand Down Expand Up @@ -189,10 +188,6 @@ func (e *EVM) SendRawTransaction(ctx context.Context, data []byte) (common.Hash,
return common.Hash{}, err
}

if tx.Gas() > txMaxGasLimit {
Copy link
Member

Choose a reason for hiding this comment

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

Why is this check removed?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

return common.Hash{}, errs.NewTxGasLimitTooHighError(txMaxGasLimit)
}

if err := models.ValidateTransaction(tx, e.head, e.evmSigner, e.validationOptions); err != nil {
return common.Hash{}, err
}
Expand Down Expand Up @@ -333,7 +328,7 @@ func (e *EVM) EstimateGas(
passingGasLimit uint64 // lowest-known gas limit where tx execution succeeds
)
// Determine the highest gas limit that can be used during the estimation.
passingGasLimit = blockGasLimit
passingGasLimit = models.TxMaxGasLimit
if tx.Gas >= gethParams.TxGas {
passingGasLimit = tx.Gas
}
Expand Down Expand Up @@ -473,7 +468,7 @@ func (e *EVM) getBlockView(
evm.StorageAccountAddress(e.config.FlowNetworkID),
e.registerStore,
blocksProvider,
blockGasLimit,
models.TxMaxGasLimit,
)

return viewProvider.GetBlockView(height)
Expand Down
21 changes: 21 additions & 0 deletions tests/web3js/debug_traces_test.js
Original file line number Diff line number Diff line change
Expand Up @@ -656,4 +656,25 @@ it('should retrieve call traces', async () => {
)
assert.equal(updateTrace.value, '0x0')
assert.equal(updateTrace.type, 'CALL')

// test that `gas` limit does not exceed the `50M` value
traceCall = {
from: conf.eoa.address,
to: contractAddress,
data: callData,
value: '0x0',
gasPrice: web3.utils.toHex(conf.minGasPrice),
gas: '0x6CB8080'
}
response = await helpers.callRPCMethod(
'debug_traceCall',
[traceCall, 'latest', callTracer]
)
assert.equal(response.status, 200)
assert.isDefined(response.body)

assert.equal(
response.body.error.message,
'gas limit is bigger than max gas limit allowed 114000000 > 50000000'
)
})
52 changes: 50 additions & 2 deletions tests/web3js/eth_failure_handling_test.js
Original file line number Diff line number Diff line change
Expand Up @@ -6,16 +6,64 @@ const web3 = conf.web3
it('should fail when tx gas limit higher than the max value', async () => {
let receiver = web3.eth.accounts.create()

let signedTx = await receiver.signTransaction({
from: receiver.address,
to: conf.eoa.address,
value: 10,
gasPrice: conf.minGasPrice,
gasLimit: 51_000_000, // max tx gas limit is 50_000_000
})
let response = await helpers.callRPCMethod(
'eth_sendRawTransaction',
[signedTx.rawTransaction]
)
assert.equal(200, response.status)
assert.isDefined(response.body)

assert.include(
response.body.error.message,
'tx gas limit exceeds the max value of 50000000'
)
})

it('should fail when eth_call gas limit higher than the max value', async () => {
let receiver = web3.eth.accounts.create()

try {
await helpers.signAndSend({
await web3.eth.call({
from: conf.eoa.address,
to: receiver.address,
value: 10,
gasPrice: conf.minGasPrice,
gasLimit: 51_000_000, // max tx gas limit is 50_000_000
})
} catch (e) {
assert.include(e.message, 'tx gas limit exceeds the max value of 50000000')
assert.include(
e.message,
'gas limit is bigger than max gas limit allowed 51000000 > 50000000'
)
return
}

assert.fail('should not reach')
})

it('should fail when eth_estimateGas gas limit higher than the max value', async () => {
let receiver = web3.eth.accounts.create()

try {
await web3.eth.estimateGas({
from: conf.eoa.address,
to: receiver.address,
value: 10,
gasPrice: conf.minGasPrice,
gasLimit: 51_000_000, // max tx gas limit is 50_000_000
})
} catch (e) {
assert.include(
e.message,
'gas limit is bigger than max gas limit allowed 51000000 > 50000000'
)
return
}

Expand Down
Loading