internal/ethapi: eth_estimateGas should take block number

[kumavis]

System information

Geth version: Geth/v1.4.3-stable/linux/go1.5.1
OS & Version: Linux

Summary

behaviour of eth_estimateGas changed

1. can no longer estimate gas of a simple value transfer tx. while a simple value transfer has a fixed cost, i normally just dump tx params here for any kind of tx to get the estimated cost) - now i’ll need to fork the logic on the frontend based on whether or not the receiving address has code - unnecesary complexity, geth should handle this case just as it used to.
2. eth_estimateGas params are, according to the wiki, supposed to match the eth_call params, and take a blockTag. I don’t use this feature other than setting it to “pending” but now it throws saying wrong arg count. I don't really care how this one is resolved, but this is a breaking change that broke metamask.

Steps to reproduce the behaviour

rejection of simple value transfer

curl -d '{"id": 1, "method":"eth_estimateGas","params": [{"to":"0x5fda30bb72b8dfe20e48a00dfc108d0915be9bb0","value":"0x1234"}]}' -X POST https://mainnet.infura.io

rejection of blockTag

curl -d '{"id": 1, "method":"eth_estimateGas","params": [{"to":"0x5fda30bb72b8dfe20e48a00dfc108d0915be9bb0","value":"0x1234"}, "latest"]}' -X POST https://mainnet.infura.io

confirm geth version

curl -d '{"id": 1, "method":"web3_clientVersion","params": []}' -X POST https://mainnet.infura.io

[obscuren]

1. dup of eth/api: eth_estimateGas broken for non-contract recipients #2577
2. the specification should change. It's a bit pointless to execute against anything other than the pending state.

[kumavis]

1. whoops, i did a quick initial search for duplicates but missed it
2. agree

[kumavis]

reopening bc you should match the spec in the meantime -- even if you ignore the blockTag parameter.

[karalabe]

The first issue should be fixed by #2589. Could you please check if it sorts out the issue for you?

[kumavis]

@karalabe yes that resolves (1)
remaining issue is (2) eth_estimateGas params does not match spec per wiki

this can be resolved by changing the spec, or allowing the optional blockTag

[lastperson]

Dunno if it should be here or not, please point me in the right direction.
Currently estimateGas works against the clean pending block, doesn't taking into account even the pending transactions from the same address. This behavior renders it impossible to check if your transaction will be successful(or estimate gas) after your previous transaction, while the previous transaction is still pending. Currently I mitigate this by waiting for the significant transaction to be mined first, then executing estimations and calls for the next one. Probably it is correct to call the default defaultBlock for calls and estimateGas latest, and use pending optionally if you want to take into account your pending tx's.

[karalabe]

It should take into account all pending transactions in the particular block. There's little sense to use the latest block for it. AFAIK pending was always the designed use case. Whay Geth version are you using? Can you give us an example of what it does vs. what it should do?

[lastperson]

I'm using the 1.4.5, will prepare an example now. Also all pending transactions is not quite safe, because you can't be sure which transactions will be mined before yours, but you can always be sure that your own transactions will be mined in sequence.

[lastperson]

@karalabe

contract PendingCall {
    bool step = false;
    function next() returns(bool) {
        step = !step;
        return step;
    }
}

var address = web3.eth.accounts[0];
var pendingcall = web3.eth.contract([{"constant":false,"inputs":[],"name":"next","outputs":[{"name":"","type":"bool"}],"type":"function"}])
  .at('0x3a6e732c55b7d576c84d65ae0ad02b9bb913006a');
pendingcall.next.call(function(err, afterFirstTx) {
  console.log("After first tx state: " + afterFirstTx);
  pendingcall.next({from: address, gas: 50000}, function(err, tx) {
    console.log("Tx: " + tx);
    var check = function() { 
      pendingcall.next.call(function(err, afterSecondTx) {
        console.log("After second tx state: " + afterSecondTx);
        if (afterFirstTx === afterSecondTx) {
          // Always getting this on the network.
          console.log("Call didn't take into account pending tx, simulated result is the same as for pending tx.");
        } else {
          // Always getting this on testrpc, because mining is immediate there.
          console.log("Call took into account pending tx(or it was mined blazingly fast), simulated result is the same as starting.");
        }
      });
    };
    setTimeout(check, 2000); // Just to make sure there is no race condition in the geth.
  });
});

Please try it. If it is not self explanatory I will do my best to explain.

[lastperson]

Btw, if we alter second call in this way:

pendingcall.next.call('pending', function(err, afterSecondTx) {

it will start work as expected.
The problem is that estimateGas doesn't take defaultBlock parameter.