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

Non deterministic gas usage of the same function on truffle development server #1012

Closed
boorac opened this issue Jun 14, 2018 · 7 comments
Closed

Comments

@boorac
Copy link

boorac commented Jun 14, 2018

Environment

node 10.0.0
pragma solidity ^0.4.21;

Issue

I'm calling the claimRefund function on my smart contract using metamask. When calling the function with 37081 gas limit at 20 gwei gas price, the transaction spends 36924 gas and gets reverted with EVM Revert.

If I change params to 58080 gas limit at 20 gwei gas price, the transaction passes spending 24721 gas.

The code is shown below:

   /**
   * @param investor Investor address
   */
  function refund(address investor) public {
    require(state == State.Refunding);
    uint256 depositedValue = deposited[investor];
    deposited[investor] = 0;
    investor.transfer(depositedValue);
    emit Refunded(investor, depositedValue);
  }
  }


  function claimRefund() public {
    require(isFinalized);
    require(!goalReached());

    vault.refund(msg.sender);
  }

Question

How is it possible that the same call results in a different gas spent amount? Also, how can a function fail after spending more gas and pass after spending less gas?

@cgewecke
Copy link
Contributor

@boorac Just for clarification, are you connecting to ganache-cli? If so which version?

I believe there's been a known problem in the ethereumjs-vm with deleting storage (e.g. setting it to zero) - refunds haven't been handled gracefully. Might have been fixed recently though.

@seesemichaelj Does this issue ring any bells for you?

@mikeseese
Copy link
Contributor

@boorac What is likely happening here is you're receiving a refund from the EVM.

There are two reasons for a refund (below are excerpts from the EVM Yellow Paper):

  • when the storage value is set to zero from non-zero (refund of 15,000 gas)
  • for self-destructing an account (refund of 24,000 gas)

It looks like deposited[investor] = 0 qualifies for the first.

Your transaction is going to take X gas to execute, at the end of the executing the transaction, if you qualify for refunds, the account is refunded the gas. Unfortunately, it's reported as "spending 24721 gas" rather than reported as "consumed 39721 gas and received a refund of 15000 gas, netting your account 24721 gas used." I would hypothesize that if you set your gas limit to 39721, it would succeed.

I'm not sure if anything is actually being done to handle this more gracefully. ethereumjs-vm (the EVM implementation that (G|g)anache(-cli|-core) uses) was updated to report how much was refunded ethereumjs/ethereumjs-monorepo#284 such that ganache could add it in the estimate provided when eth_estimateGas was called (still waiting on merge trufflesuite/ganache#80) so that the estimate would include how much was actually needed to execute the transaction successfully.

Hope this helps clear things up.

@mikeseese
Copy link
Contributor

With all that said, I'm not sure if/where the issue is. Some questions to ponder:

  • Does Metamask call eth_estimateGas to determine the suggested gas limit for a transaction?
  • If it does (rather than doing it's own estimation), then the follow up question would be what blockchain are you using to do this? I would suspect you're using ganache with truffle migrate.
  • If so, then the issue lies in solving estimateGas returns invalid value when tx has a gasRefund ganache#26 which requires some discussion about how eth_estimateGas is perceived (which may have recently been discussed; I'll have to ping @benjamincburns about that) so we can pick an implementation

@boorac
Copy link
Author

boorac commented Jun 15, 2018

@cgewecke I'm using the command truffle develop, not sure which cli is behind the abstraction, is there a way to find out?

@seesemichaelj I understand the issue better now, thank you. Regarding Metamask and eth_estimateGas, I suspect that Metamask uses some kind of heuristic since every time I attempt to call a function which will be reverted the gasLimit in the transaction is automatically set to 0.

I will try sniffing out the traffic using wireshark and report back. Also, if Metamask does indeed use eth_estimateGas, I'm wondering why is it making the wrong estimation, that is, chooses an incorrect gas limit. Is eth_estimateGas deterministic in nature?

@cgewecke
Copy link
Contributor

@boorac Yes truffle develop runs ganache-cli.

It looks like MetaMask runs estimateGas and adds a gas buffer of 50% or runs the tx at 90% of the block gas limit if the buffer is too high.

@mikeseese
Copy link
Contributor

Per @cgewecke's comment, this means that trufflesuite/ganache#26 is causing the issue you're seeing @boorac

@cgewecke
Copy link
Contributor

@boorac I'm going close here and we'll track this over at ganache-core 26

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

No branches or pull requests

3 participants