Skip to content
This repository has been archived by the owner on Jan 21, 2022. It is now read-only.

estimateGas hits an out-of-gas error for a transaction that works with hardcoded gas #86

Closed
niran opened this issue May 9, 2016 · 13 comments · Fixed by MetaMask/web3-provider-engine#78

Comments

@niran
Copy link

niran commented May 9, 2016

I'm trying to send a transaction with web3.js, which I believe does an internal call to estimate gas before sending the transaction. During that estimateGas call, TestRPC errors with VM Exception while executing eth_estimateGas: out of gas. If I hardcode the gas to 188561, TestRPC mines the transaction successfully. I suspect a different gas limit is being used for estimation and action transaction processing.

The transaction I'm using is a contract deployment with this data:

{
  "from": <any address with ether, e.g. `0x90f8bf6a479f320ead074411a4b0e7944ea8c9c1` for `testrpc -d`>,
  "data": "606060405260008054600160a060020a031916331790556101ab806100246000396000f3606060405260e060020a60003504631a695230811461003c5780632f54bf6e1461004b5780638da5cb5b14610066578063d7f31eb914610078575b005b61003a6004356100fc33610052565b6100cd6004355b600054600160a060020a0390811691161490565b6100df600054600160a060020a031681565b604080516020600460443581810135601f810184900484028501840190955284845261003a94823594602480359560649492939190920191819084018382808284375094965050505050505061012433610052565b60408051918252519081900360200190f35b60408051600160a060020a03929092168252519081900360200190f35b15610121576000805473ffffffffffffffffffffffffffffffffffffffff1916821790555b50565b156101a65782600160a060020a03168282604051808280519060200190808383829060006004602084601f0104600302600f01f150905090810190601f1680156101825780820380516001836020036101000a031916815260200191505b5091505060006040518083038185876185025a03f19250505015156101a657610002565b50505056"
}`
@kumavis
Copy link
Contributor

kumavis commented May 30, 2016

I suspect a different gas limit is being used for estimation and action transaction processing.

interesting hypothesis!

@kumavis
Copy link
Contributor

kumavis commented May 30, 2016

#96

@graup
Copy link

graup commented Jun 3, 2016

I'm also currently running into out of gas while executing eth_estimateGas. Shouldn't estimateGas never throw this error? Our contract does have a lot of code, but the point of testrpc should be to be able to estimate and deploy in any case, right?

As the VM throws this error, this issue should probably be in ethereumjs-vm?

@kumavis
Copy link
Contributor

kumavis commented Jun 3, 2016

@graup hmmm... yeah I would agree that its unnexpect to see estimateGas OOG... I think this is a problem with the estimateGas implementation, meaning its over in provider-engine.

Guess we can set some crazy high gasLimit there, then its up to the rpc method consumer to determine whether or not that gas usage is safe e.g. below the block limit

That said, we may need a flag in vm to skip the gasLimit vs sender account balance check

@graup
Copy link

graup commented Jul 8, 2016

I remember there were some more comments in this issue, and someone pointed to MetaMask/web3-provider-engine#76 which has since been deleted. Can someone check if this issue has been fixed? I'm a little confused now.

@kumavis
Copy link
Contributor

kumavis commented Jul 12, 2016

which has since been deleted.

not sure what has been deleted? maybe you can clarify

The issue has not been resolved yet. As a fix for now you can remove the gas or gasLimit parameter from your params provided to eth_estimateGas

@graup
Copy link

graup commented Jul 12, 2016

@kumavis The link I mentioned 404'd. All your other comments here were hidden, too! Now everything is back. Thanks!

@tcoulter
Copy link
Contributor

I'm looking into this. One big thing while I determine if this is an error:

I'm also currently running into out of gas while executing eth_estimateGas. Shouldn't estimateGas never throw this error?

It should throw this error. If it doesn't, this leaves nodes open to a DoS attack. If you get an out of gas issue during estimateGas, then it's likely your request uses more than the available gas. That, or there's a bug.

I'm working now to determine if there is actually a bug. I'll do so by trying to fill up the gas limit to the max available (0x47E7C4). If I can get to the max in a simple contract using estimateGas, then there's likely no issue here.

@kumavis
Copy link
Contributor

kumavis commented Jul 12, 2016

I agree with @tcoulter that an OOG is appropriate in certain situations. now the most important question is "what does geth go"

@tcoulter
Copy link
Contributor

@niran One of the important questions we should have asked what: What is the solidity code related to this call? Do you still have it? It's very likely that the constructor does, in fact, run out of gas.

@tcoulter
Copy link
Contributor

Hmm, scratch that. Would help if I read your initial writeup better.

@tcoulter
Copy link
Contributor

tcoulter commented Jul 12, 2016

Alright, so just figured this out. The issue is that your data field's value is not prefixed with 0x. If you prefix it with 0x, estimate gas works correctly.

I wrote a test that you can verify yourself. Stick this in the TestRPC's test/request.js file, in the tests() function. Remove the 0x from the data field, and you'll see that it fails with an OOG error.

  describe("gas limit", function() {
    it.only("aaa", function(done) {
      web3.currentProvider.sendAsync({
        "method": "eth_estimateGas",
        "params": [{
          "from": accounts[0],
          "data": "0x606060405260008054600160a060020a031916331790556101ab806100246000396000f3606060405260e060020a60003504631a695230811461003c5780632f54bf6e1461004b5780638da5cb5b14610066578063d7f31eb914610078575b005b61003a6004356100fc33610052565b6100cd6004355b600054600160a060020a0390811691161490565b6100df600054600160a060020a031681565b604080516020600460443581810135601f810184900484028501840190955284845261003a94823594602480359560649492939190920191819084018382808284375094965050505050505061012433610052565b60408051918252519081900360200190f35b60408051600160a060020a03929092168252519081900360200190f35b15610121576000805473ffffffffffffffffffffffffffffffffffffffff1916821790555b50565b156101a65782600160a060020a03168282604051808280519060200190808383829060006004602084601f0104600302600f01f150905090810190601f1680156101825780820380516001836020036101000a031916815260200191505b5091505060006040518083038185876185025a03f19250505015156101a657610002565b50505056"
        }],
        jsonrpc: "2.0",
        id: 27
      }, function(e, r) {
        console.log(e, r);
        done();
      })
    });
  });

@kumavis
Copy link
Contributor

kumavis commented Jul 12, 2016

really this is etheruemjs-tx's fault for ever accepting utf8 as a valid encoding mechanism for string

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants