3

Pardon me if this question seems a bit naive. I have written a smart contract (for proof of existence) and simulating it in Ganache. Inside it I have logic to store data in a mapping created between index and a struct as below:

/// mapping of document and index
mapping (uint => Proof) idToProof;

// Struct 
struct Proof {
    uint16   id;
    uint     proofdate;
    address  creator; 
    string   title;
    string   proofhash;
    string   ipfshash;

}

My contract runs into out of gas exception when the line of code related to store data in the mapping is executed (highlighted below):

 function _saveTheProof(uint _proofTimeStamp) private {    
     Proof memory newProof = Proof(proofId, proofTimeStamp, proofCreator, proofTitle, proofHash, proofIpfsHash); 
     idToProof[proofId] = newProof; // <-- this line
     newProofCreated(proofId, proofCreator, proofTitle, proofHash, proofIpfsHash, proofRemarks, proofTags, _proofTimeStamp);
} 

When I remove this code my dapp works perfectly fine. I am really puzzled what is the issue here, why does Gas computed by Metamask is insufficient for the execution of this logic. If Gas needs computed by Metamask are nearly correct then why do we end up having Out of Gas error.

Please help correct my concepts here or if my code is to be blamed here.

  • I think MetaMask should call eth_estimateGas() and then increase the returned gas estimation by 20% , this should be more than enough to cover all the unxpected behaviour of the contract. – Nulik Aug 10 '18 at 20:17
3

Gas estimation is not an exact science. There are certain cases when the gas estimate can be wrong, or impossible to estimate properly.

Certain such cases depend on behaviour. If a contract has different code paths triggered by variables not controlled by the tx (say, the block hash), then an estimate and actual run might take different amounts of gas. For instance, if the blockhash is odd, you might consume 30000 gas, and 50000 if it is even. If the blockhash is odd when gas is estimated, but even when it is mined, the tx will fail.

Another common scenario relates to storage. You get a "gas refund" if your transaction frees up storage. So if a transaction consumed 50000 gas, and gets a 2000 gas refund, estimate gas will return 48000, based on the final cost. However, the transaction will still need a gas limit of 50000 to succeed, since it needs to finish computation before any refunds can be applied. This will result in a failing transaction if the gas limit is not manually increased.

When dealing with arrays and mappings, similar errors can occur. At least some time ago, systems like metamask were unable to correctly handle array traversal estimations, I'm not sure if that is still the case.

Try setting a very high gas limit, and seeing a trace difference between the failing and the succeeding transaction to see if you're running into any operations that might result in a gas refund, or some kind of variable logic or array traversal.

  • Thanks @Raghav Sood for your reply. Given this much degree of uncertainty then how can one create an efficient dapp and smart contract design without frustrating an end user. Making hit and trials by entering different Gas values in Metamask will obviously be a very bad experience and will discourage people from using the dapp. Are there any practical solutions or best practices to reduce this problem if not solve it completely ? – tanmay Aug 10 '18 at 19:48
  • @tanmay As a developer, you can calculate an expected gas price on non-trivial functions, and set the gas limit yourself before passing control to metamask. – Raghav Sood Aug 10 '18 at 20:04
2

Metamask's gas calculation is not perfect at all. The core of the problem lies in the eth_EstimateGas function that is simply running a contract and obtains usedGas value.

This is how it does it in /internal/ethapi/api.go:

// Create a helper to check if a gas allowance results in an executable transaction
executable := func(gas uint64) bool {
    args.Gas = hexutil.Uint64(gas)

    _, _, failed, err := s.doCall(ctx, args, rpc.PendingBlockNumber, vm.Config{}, 0)
    if err != nil || failed {
        return false
    }
    return true
}
// Execute the binary search and hone in on an executable gas limit
for lo+1 < hi {
    mid := (hi + lo) / 2
    if !executable(mid) {
        lo = mid
    } else {
        hi = mid
    }
}

This function uses a binary search to find best gas price, however this heuristics is completely useless. Why? Because of the gas refunds. If your contract returns storage back a negative gas usage occurs. For example if you spend 100,000 gas and clear a lot of storage you may receive 40,000 gas back in refunds. But eth_EstimateGas will return a much lower value than 100,000 because eth_EstimateGas thinks that the correct gas limit is 60,000. Since the refunds are processed only at the end of the Call() you have to provide the maximum gas limit to survive until the end.

Refunds are processed internally in the ApplyMessage function, and eth_EstimateGas has no way to know if there was a refund or not. This is why, whatever you use, binary search or Black Scholes model to estimate gas limit, you are going to get it wrong anyway.

If you want to fix this problem, just get the real gas consumed by the contract from ApplyMessage function. Modify original get sources to return this variable:

st.gasUsed()

This is how I do it in my own custom modified geth:

func (st *StateTransition) TransitionDb() (ret []byte, usedGas,maxUsedGas uint64, failed bool, err error) {
    if err = st.preCheck(); err != nil {
        return
    }
    msg := st.msg
    sender := vm.AccountRef(msg.From())
    homestead := st.evm.ChainConfig().IsHomestead(st.evm.BlockNumber)
    contractCreation := msg.To() == nil

    // Pay intrinsic gas
    gas, err := IntrinsicGas(st.data, contractCreation, homestead)
    if err != nil {
        return nil, 0,0, false, err
    }
    if err = st.useGas(gas); err != nil {
        return nil, 0,0, false, err
    }
    var (
        evm = st.evm
        // vm errors do not effect consensus and are therefor
        // not assigned to err, except for insufficient balance
        // error.
        vmerr error
    )
    if contractCreation {
        ret, _, st.gas, vmerr = evm.Create(sender, st.data, st.gas, st.value)
    } else {
        // Increment the nonce for the next transaction
        st.state.SetNonce(msg.From(), st.state.GetNonce(sender.Address())+1)
        ret, st.gas, vmerr = evm.Call(sender, st.to(), st.data, st.gas, st.value)
    }

    if vmerr != nil {
        log.Debug("VM returned with error", "err", vmerr)
        // The only possible consensus-error would be if there wasn't
        // sufficient balance to make the transfer happen. The first
        // balance transfer may never fail.
        if vmerr == vm.ErrInsufficientBalance {
            return nil, 0,0, false, vmerr
        }
    }
    maxUsedGas=st.gasUsed()
    st.refundGas()
    st.state.AddBalance(st.evm.Coinbase, new(big.Int).Mul(new(big.Int).SetUint64(st.gasUsed()), st.gasPrice))

    return ret, st.gasUsed(),maxUsedGas, vmerr != nil, err
}

Obviously, you have to propagate the additional returned value upwards to all the functions that call ApplyMessage, but this is not a big work.

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.