Skip to content
This repository has been archived by the owner on Feb 26, 2024. It is now read-only.

Use eth_getEstimate to get the gas limit for Optimistic Ethereum #4217

Closed
qbzzt opened this issue Aug 2, 2021 · 25 comments
Closed

Use eth_getEstimate to get the gas limit for Optimistic Ethereum #4217

qbzzt opened this issue Aug 2, 2021 · 25 comments
Labels
L2 Issues Related to layer two support inside Truffle

Comments

@qbzzt
Copy link

qbzzt commented Aug 2, 2021

Issue

In Optimistic Ethereum we use eth_getEstimate from the server to return an estimate of the cost of a transaction. We do this because the cost of a transaction is not primarily the cost of execution, but the cost of L1 storage. See https://community.optimism.io/docs/developers/fees.html#fees-for-sequencer-transactions.

This clashes with Truffle's standard algorithm of looking at the gas cost of a transaction internally and setting gasLimit to a multiple of that value. This causes a failure when attempting to deploy a contract or run a test on Optimistic Kovan (running with the internal development node is not an issue).

Steps to Reproduce

  1. In an empty directory, run

    truffle unbox optimism
  2. Create a .env file with this content:

    KOVAN_MNEMONIC=   <mnemonic for an account that has enough Kovan ETH to deploy>
    INFURA_KEY=   <your Infura key> 
    
  3. Test the sample dapp:

    truffle test --config truffle-config.ovm.js --network optimistic_kovan
    

Results

  • Without anything else the migration fails with:

    Compiling your contracts...
    ===========================
    > Compiling ./contracts/optimism/SimpleStorage.sol
    > Artifacts written to /tmp/test--1434-S5SAsFHe1W7m
    > Compiled successfully using:
       - solc: 0.7.6
    
    Error: while migrating SimpleStorage: fee too high: 494550000000000, use less than 27300000000000 * 0.800000
        at /usr/lib/node_modules/truffle/build/webpack:/packages/deployer/src/deployment.js:365:1
        at processTicksAndRejections (internal/process/task_queues.js:97:5)
        at Migration._deploy (/usr/lib/node_modules/truffle/build/webpack:/packages/migrate/Migration.js:75:1)
        at Migration._load (/usr/lib/node_modules/truffle/build/webpack:/packages/migrate/Migration.js:61:1)
        at Migration.run (/usr/lib/node_modules/truffle/build/webpack:/packages/migrate/Migration.js:218:1)  
        at Object.runMigrations (/usr/lib/node_modules/truffle/build/webpack:/packages/migrate/index.js:150:1)
        at Object.runFrom (/usr/lib/node_modules/truffle/build/webpack:/packages/migrate/index.js:110:1)
        at Object.runAll (/usr/lib/node_modules/truffle/build/webpack:/packages/migrate/index.js:114:1)
        at Object.run (/usr/lib/node_modules/truffle/build/webpack:/packages/migrate/index.js:79:1)
        at Object.run (/usr/lib/node_modules/truffle/build/webpack:/packages/core/lib/testing/Test.js:109:1)
        at Object.run (/usr/lib/node_modules/truffle/build/webpack:/packages/core/lib/commands/test/index.js:160:1)
        at Command.run (/usr/lib/node_modules/truffle/build/webpack:/packages/core/lib/command.js:167:1)
    Truffle v5.4.0 (core: 5.4.0)
    Node v12.22.
    
  • We can fix this problem by editing migrations/1_deploy_contracts.js to:

    var SimpleStorage = artifacts.require("SimpleStorage");
    
    module.exports = function(deployer) {
      deployer.deploy(SimpleStorage, { gasPrice: 15000000, gas: 1820000 });
    };

    But then we get this error:

    Compiling your contracts...
    ===========================
    > Compiling ./contracts/optimism/SimpleStorage.sol
    > Artifacts written to /tmp/test--1487-EURBlcvlxR0c
    > Compiled successfully using:
       - solc: 0.7.6
    
    
    
      Contract: SimpleStorage
        1) ...should store the value 89.
        > No events were emitted
    
    
      0 passing (1s)
      1 failing
    
      1) Contract: SimpleStorage
           ...should store the value 89.:
         fee too high: 494550000000000, use less than 12600000000000 * 0.800000
    
    
  • We can also fix this error by modifying the code to specify a gas limit in test/simplestorage.js. However, manually
    specifying a gas limit in every transaction in every test is bad user experience.

@fainashalts
Copy link
Contributor

Hi @qbzzt thanks for bringing this up! Truffle estimates gas and the default gasMultiplier is 1.25. I think this multiplier isn't sufficient for our needs here.

What do you think about setting the gasMultiplier on a per-contract basis? This should address the issues in tests and anywhere else the contract is used. It would look something like this:

const SimpleStorage = artifacts.require("SimpleStorage");
SimpleStorage.gasMultiplier = 1.5;

This would override the default multiplier. If I remember right, MetaMask upped their multiplier to 1.5 and that was enough, though we could play around with the right number.

We don't have an easy way to set this at the configuration level though that might be something we can do in the future.

Let me know what you think, thanks!

@qbzzt
Copy link
Author

qbzzt commented Aug 4, 2021

I think this is sufficient for deployments to Optimistic Kovan, where the gas is essentially free. Would you be OK if I documented this (after you do it, of course) and left a note that it would probably be best to use a different system for deploying for the production network when you get to it? We charge gasLimit * gasPrice, rather than gasUsed * gasPrice, so this isn't a good idea in production.

@fainashalts
Copy link
Contributor

Hi @qbzzt I'd really like to help make it feasible to deploy from Truffle to the production chain! Why do you charge gasLimit * gasPrice rather than gasUsed * gasPrice and do you have a suggestion for how we could do this better from the Truffle side? What would the ideal flow look like here?

@qbzzt
Copy link
Author

qbzzt commented Aug 4, 2021

gasUsed is a tiny portion of the cost of an Optimistic Ethereum transaction. The real cost is the storage on L1 (see https://community.optimism.io/docs/developers/fees.html). And just having a higher multiplier isn't going to do much good, because real gas costs real money.

If we can't do an automatic call to eth_estimateGas as part of the deployment, have two options, both requiring a somewhat more complicated deployment code:

  1. Call eth_estimateGas "manually" before the deployment transaction
  2. Call an L1 endpoint to get the current cost of gas, and estimate the cost ourselves based on code side and the L1 gas cost.

Either is possible, but neither is great developer experience. It might be better to just create a web page where people can use a wallet to deploy their code.

@fainashalts
Copy link
Contributor

Hi @qbzzt sorry for the delay! I will do a deeper dive into this issue this week to see what we can do on our side.

When you say an automatic call to eth_estimateGas, what do you mean? Where would you want this call to happen?

I think I'm a bit confused about how gas for a deployment to Optimistic L2 is calculated -- you say gasUsed * gasPrice but that is for the L2, and then we need to account for gas on L1 as well? Is there a clear chunk of code somewhere you could point me toward? I just want to be sure I'm seeing the whole picture here so we can support the flow in the best way possible. Thanks!

@qbzzt
Copy link
Author

qbzzt commented Aug 9, 2021

@fainashalts , no problem. I prefer late answers to hurried ones.

gasUsed * gasPrice is in L1. In L2 it is documented here: https://community.optimism.io/docs/developers/fees.html#under-the-hood , except I need to change the formula to:

total_cost = (transaction_size_in_bytes * data_price) + (execution_gas_limit * execution_price)

We need to account for gas cost of L1 for the storage. That is the main cost of the transaction, and contract creation transactions require a lot of storage (one byte for each byte of the code, at least).

Here is what happens currently:

  1. My user code instantiates a new contract:

    storage = await SimpleStorage.new()
  2. Inside SimpleStorage.new() Truffle performs some steps:
    a. Read the SimpleStorage.json file to get the contract's bytecode
    b. Create a transaction to address 0 to create the contract
    c. Send that transaction, and receive a Promise object
    d. Return the Promise object.

  3. My user code decides what to do with that Promise.

What I want is to have a step between b and c that runs eth_estimateGas on the transaction, and then modify the transaction's gasLimit to the amount returned by eth_estimateGas.

@fainashalts
Copy link
Contributor

Hi @qbzzt,

Just wanted to provide an update -- I discovered that we use a hard-coded default gas when one is not provided, which is definitely not ideal, regardless of the chain or layer involved. I am working to find the best way to change this that addresses all the places where this default is currently used.

In the meantime, I got to thinking -- when I deploy to the Optimistic L2 from Truffle, I am using web3 to get certain information from the chain. I think web3 basically assumes I'm just talking to the L1, but you all have a fork of the @ethereumjs/vm package, right?

When I estimate gas on an optimistic chain, what exactly am I getting back? I am just wondering if I should be treating this gas estimate the same way as I do the Ethereum one or if I should be trying to inject the context of which chain I am on into this estimation (my initial impulse was not to inject context at this stage and just treat the estimate the same way). Would love to know your thoughts/findings on this!

@qbzzt
Copy link
Author

qbzzt commented Aug 17, 2021

I don't think we use a fork of @ethereumjs/vm, because it won't help. It can give us the L2 gas required, but that value is just a small part of the story. The major cost is L1 storage, which depends on L1 gas cost. That's why we need to ask an actual node with eth_estimateGas (which returns an estimate of the overall cost when going to an Optimistic Ethereum node)

@fainashalts
Copy link
Contributor

fainashalts commented Aug 17, 2021

Sorry @qbzzt, I am pretty sure I saw a fork of @etherueumjs/vm somewhere in your code but it was several months ago before the switch to a monorepo and I can't find it now. I could definitely be mistaken!

I can ask the node I am connected to for the gas using eth_estimateGas, but my question from the previous comment stands:

When I estimate gas on an optimistic chain, what exactly am I getting back? I am just wondering if I should be treating this gas estimate the same way as I do the Ethereum one or if I should be trying to inject the context of which chain I am on into this estimation (my initial impulse was not to inject context at this stage and just treat the estimate the same way). Would love to know your thoughts/findings on this!

The reason I got to thinking about this is that when I hit the L2 node with an estimateGas request to set the gas, I get an exceeds blockLimit error and that does not happen when I test with the L1 Kovan node. Am I missing something? I see @eth-optimism/core-utils has a TxGasLimit that it exposes, should I incorporate it into the Truffle config file for Optimism? Or is there something else going on? Thanks for any clarification you can provide here!

@qbzzt
Copy link
Author

qbzzt commented Aug 18, 2021

I can ask the node I am connected to for the gas using eth_estimateGas, but my question from the previous comment stands:

When I estimate gas on an optimistic chain, what exactly am I getting back? I am just wondering if I should be treating this gas estimate the same way as I do the Ethereum one or if I should be trying to inject the context of which chain I am on into this estimation (my initial impulse was not to inject context at this stage and just treat the estimate the same way). Would love to know your thoughts/findings on this!

Sorry I didn't answer it. When you ask for a gas estimate it gives you the equation in https://community.optimism.io/docs/developers/fees.html#encoding-sequencer-transaction-costs. It's fairly involved, which is why it's best to just rely on eth_estimateGas.

The reason I got to thinking about this is that when I hit the L2 node with an estimateGas request to set the gas, I get an exceeds blockLimit error and that does not happen when I test with the L1 Kovan node. Am I missing something?

You mean when you try to then send a transaction with that gasLimit to Optimistic Kovan? It might be related to ethereum-optimism/optimism#1081.

I see @eth-optimism/core-utils has a TxGasLimit that it exposes, should I incorporate it into the Truffle config file for Optimism? Or is there something else going on? Thanks for any clarification you can provide here!

TxGasLimit is a way to get the estimate without having to go through eth_estimateGas, but it requires a lot of information. It's a lot better to just use eth_estimateGas when possible.

@fainashalts
Copy link
Contributor

Hi @qbzzt! My apologies, it looks like I originally misunderstood what you were asking for here. To be clear, you are requesting that we change the way we get the gasLimit for a transaction?

For the sake of absolute clarity, in Truffle land:

gas is the most you are willing to pay for a transaction, and
gasLimit/blockLimit is the most gas that can be spent in a block (not sure how that translates to Optimism, so herein may be the problem?)

We check for the gasLimit in the Truffle code when it is not provided within the config, here. We do this by getting the latest block from web3, which contains the gasLimit, here.

Re-reading our correspondence I see you meant you'd like us to use the estimate_gas value to be the gasLimit you get back (I guess ethers.js calls gasLimit gas and that may have led to some of my confusion...). After digging through our code I see there are some inconsistencies in naming the gas variables that may be relevant here, so I'm going to spend some time working it out to find the best way to address this issue. I'll keep you posted!

@qbzzt
Copy link
Author

qbzzt commented Aug 20, 2021

IIRC in Optimism each transaction is its own block, so the two figures are really the same.

Sorry I wasn't clear on what I meant earlier.

@qbzzt
Copy link
Author

qbzzt commented Aug 20, 2021

Here's where I am now. I'm trying to use <contract>.new.estimateGas() and use a multiple of that, and it still fails:

truffle(optimistic_kovan)> await SimpleStorage.new.estimateGas(0)
3000045
truffle(optimistic_kovan)> x = await SimpleStorage.new(0, {gas: 3000045*2, gasLimit: 300045*3, gasPrice: 150000000})
Uncaught:
{
  code: -32000,
  message: 'invalid transaction: exceeds block gas limit'
}

When I try to run estimateGas on mainnet, it fails with an allowance error, even though it shouldn't cost anything as a read only transaction:

truffle(optimistic_mainnet)> await SimpleStorage.new.estimateGas(0)
Uncaught { code: -32000, message: 'gas required exceeds allowance (6721975)' }

@fainashalts
Copy link
Contributor

Hi @qbzzt thanks for keeping me updated! Yes, I get the exceeds blockLimit error as well when setting gas to <contract>.new.estimateGas() in the deployment flow. That has been the main issue I've been trying to debug lately.

I just tried it and I do see this error using estimateGas in optimistic mainnet as well. 6721975 is the default gas value that is hardcoded, as I mentioned. But you're right that shouldn't happen since it shouldn't cost anything to read that info. I tried increasing gas to an astronomical # and still got the error. A bit of searching around yielded that this error may actually just be obscuring the other error re: exceeding the blockLimit.

Neither of these errors happen when tested against regular Kovan and regular Ethereum mainnet, so my thought is that there is a mismatch somewhere in Truffle's communication with Optimism. Is this document up to date? https://community.optimism.io/docs/protocol/evm-comparison.html

I want to be sure that whatever changes we make are consistent with how things are done on Ethereum as well, since ideally devs shouldn't have to get in the weeds on this sort of thing when developing for either chain.

A couple of us over at the Truffle team will be meeting about the entire gas/gasPrice/gasLimit/blockLimit flow next week, and we may be reworking it to be more straightforward. In that conversation, I'll be sure we consider the Optimism needs and try to address everything we've talked about here. Any additional info you'd like to provide for that in the meantime would be appreciated!

@qbzzt
Copy link
Author

qbzzt commented Aug 20, 2021

Neither of these errors happen when tested against regular Kovan and regular Ethereum mainnet, so my thought is that there is a mismatch somewhere in Truffle's communication with Optimism. Is this document up to date? https://community.optimism.io/docs/protocol/evm-comparison.html

It's accurate for right now. We are working on an upgrade that will change some things (https://community.optimism.io/docs/developers/l2/deploy.html#key-info), but I don't think it will affect the gas issue.

PS

Thank you for representing us.

@qbzzt
Copy link
Author

qbzzt commented Aug 23, 2021

It looks like our October update will remove this problem, so this is not a big priority for us.

@fainashalts
Copy link
Contributor

Oh, interesting @qbzzt! Would you mind telling/showing me what changes will be made that will address this? I want to be sure anything we change won't mess it up. :)

@qbzzt
Copy link
Author

qbzzt commented Aug 24, 2021

Of course not. We're going to do two things:

  1. Report the real L2 gasPrice and gasLimit, and charge those amounts
  2. In the next line in the node code (which we control, because it's a mutated version of geth) also charge the L1 costs.

The disadvantage is that unless we can get wallets to change their UI they'll report a wrong gas price for the transaction. The advantage, though, is that we'll be back to using the normal Ethereum method so 3rd party tools, such as Truffle, will work without modification.

@fainashalts
Copy link
Contributor

Hi @qbzzt I will be making some changes to the way we do gas estimation to ensure this sort of thing doesn't happen again. Thank you for pointing out the issue as it led to a deeper dive for me into Truffle's gas estimation flow that was really useful and informative. I will let you know when the changes are live.

In the meantime, I believe if you set gas: 0 in the network configuration in truffle-config.ovm.js that will force Truffle to do the gas estimation we want. Let me know if that helps for now!

@qbzzt
Copy link
Author

qbzzt commented Aug 27, 2021 via email

@qbzzt
Copy link
Author

qbzzt commented Aug 27, 2021

Hi @qbzzt I will be making some changes to the way we do gas estimation to ensure this sort of thing doesn't happen again. Thank you for pointing out the issue as it led to a deeper dive for me into Truffle's gas estimation flow that was really useful and informative. I will let you know when the changes are live.

In the meantime, I believe if you set gas: 0 in the network configuration in truffle-config.ovm.js that will force Truffle to do the gas estimation we want. Let me know if that helps for now!

It doesn't.

ori@Oris-MacBook-Pro dapp % truffle console --config truffle-config.ovm.js --network optimistic_kovan
truffle(optimistic_kovan)> x = await SimpleStorage.new()
Uncaught:
{
  code: -32000,
  message: 'invalid transaction: exceeds block gas limit'
}
truffle(optimistic_kovan)> 

When my optimistic_kovan definition is:

    optimistic_kovan: {
      network_id: 69,
      chain_id: 69,
      gasPrice: 15000000,
      gas: 0,
      provider: function() {
        return new HDWalletProvider(kovanMnemonic, "https://optimism-kovan.infura.io/v3/"+ infuraKey, 0, 1);
      }
    },

@fainashalts
Copy link
Contributor

Ah interesting! Setting gas: 0 forces a gas estimation from the Truffle side the way the code currently is. This means there is an additional issue to sort out once that happens. I'll try to dig in further next week and see what this may be about. Thanks for checking!

@qbzzt
Copy link
Author

qbzzt commented Aug 27, 2021 via email

@gnidan
Copy link
Contributor

gnidan commented Sep 3, 2021

I think we can close this now as duplicate of #3992, since that covers the general case beyond just Optimism.

Please follow PR #4296, which should resolve this issue altogether.

@scglenn
Copy link

scglenn commented Jan 18, 2022

This isnt resolved

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
L2 Issues Related to layer two support inside Truffle
Projects
None yet
Development

No branches or pull requests

4 participants