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.

Sign up for GitHub

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

Increase computation limit for Cadence tx #736

Merged
merged 2 commits into from
Feb 3, 2025
Merged

Increase computation limit for Cadence tx #736

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
0939865
Increase computation limit for Cadence tx
m-Peter Jan 28, 2025
b89ad63
Merge branch 'main' into mpeter/increase-flow-tx-compute-limit
franklywatson Feb 3, 2025
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: 7 additions & 0 deletions models/errors/errors.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -66,6 +66,13 @@ func NewTxGasPriceTooLowError(gasPrice *big.Int) error {
))
}

func NewTxGasLimitTooHighError(maxGasLimit uint64) error {
return NewInvalidTransactionError(fmt.Errorf(
"tx gas limit exceeds the max value of %d: ",
maxGasLimit,
))
}

func NewRecoverableError(err error) error {
return fmt.Errorf("%w: %w", ErrRecoverable, err)
}
Expand Down
9 changes: 8 additions & 1 deletion services/requester/requester.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@ import (
"github.com/onflow/flow-go/fvm/evm/emulator"
"github.com/onflow/flow-go/fvm/evm/offchain/query"
evmTypes "github.com/onflow/flow-go/fvm/evm/types"
flowGo "github.com/onflow/flow-go/model/flow"
"github.com/onflow/go-ethereum/common"
gethCore "github.com/onflow/go-ethereum/core"
"github.com/onflow/go-ethereum/core/txpool"
Expand Down Expand Up @@ -43,6 +44,7 @@ var (

const minFlowBalance = 2
const blockGasLimit = 120_000_000
const txMaxGasLimit = 50_000_000
Copy link
Contributor

Choose a reason for hiding this comment

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

I thought max gas was 30M

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Ethereum has a max gas limit of 30M, per block. However, that is not the case with Flow EVM, we are not bounded by that. This magic value I found by running EVM transactions against Testnet & Mainnet, with SetComputeLimit(9999).

With >= 51M, we get the following error:

error: [Error Code: 1300] evm runtime error: insufficient computation
   --> 8c5303eaa26202d6.EVM:529:8

Copy link

Choose a reason for hiding this comment

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

The 30M limit on Flow EVM was to keep some room for the Cadence compute units for the Cadence transaction that wraps the EVM transaction. See this FLIP - https://github.com/onflow/flips/blob/main/governance/20240508-computation-limit-hike.md under "Set Gas to compute conversion ratio ...".

Copy link
Contributor

Choose a reason for hiding this comment

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

The weights were set so that 9999 computation equates to 50M evm gas (+- rounding error).
Like Vishal mentioned this was done because we wanted to fit at least 30M evm gas into one flow transaction, but each flow transaction also has a variable computation cost coming form the cadence code (that wraps the EVM transaction).

I see nothing wrong with adding this check on the gateway, we just have to remember to change it if we change the fee weights.


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

if tx.Gas() > txMaxGasLimit {
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 @@ -589,7 +595,8 @@ func (e *EVM) buildTransaction(

flowTx := flow.NewTransaction().
SetScript(script).
SetReferenceBlockID(latestBlock.ID)
SetReferenceBlockID(latestBlock.ID).
SetComputeLimit(flowGo.DefaultMaxTransactionGasLimit)

for _, arg := range args {
if err := flowTx.AddArgument(arg); err != nil {
Expand Down
21 changes: 20 additions & 1 deletion tests/web3js/eth_failure_handling_test.js
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,8 +1,27 @@
const { assert } = require('chai')
const helpers = require('./helpers')
const conf = require("./config")
const conf = require('./config')
const web3 = conf.web3

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

try {
await helpers.signAndSend({
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')
return
}

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

it('should fail when nonce too low', async () => {
let receiver = web3.eth.accounts.create()

Expand Down
Loading