trufflesuite / ganache-cli-archive

Fast Ethereum RPC client for testing and development. See https://github.com/trufflesuite/ganache for current development.
https://www.trufflesuite.com/ganache
MIT License
3.36k stars 697 forks source link

eth_calls that would create a contract have unexpected results #149

Closed PeterBorah closed 7 years ago

PeterBorah commented 8 years ago

eth_calls that represent a transaction that would include a contract creation return nonsense data, even if their behavior is deterministic.

Expected Behavior

eth_call should have the same behavior as eth_sendTransaction, except for actually creating the transaction.

Current Behavior

eth_calls that would include the creation of a contract return nonsense data.

Possible Solution

I suspect the contract is not getting created at all, even temporarily, and the nonsense data represents what you get back from calling a non-existent contract. I'm not certain, however.

Steps to Reproduce (for bugs)

The following code fails:

contract Doubler {
  uint temp;

  function double(uint n) constant returns(uint) {
    // Setting the temp variable rather than returning directly is to demonstrate that mutating 
    // state is not the problem. eth_call works correctly with this function.
    temp = n *2;
    return temp;
  }
}

contract Delegator {
  function doubleWithTempContract(uint n) returns(uint) {
    Doubler doubler = new Doubler();
    return doubler.double(n);
  } 
} 

The failure can be showed with the following truffle test:

it("should double", function() {
  var delegator = Delegator.deployed();
  return delegator.doubleWithTempContract.call(3).
    then(function(result) {
      assert.equal(result, 6);
    });
});

Context

I have run into this in a few places. The most recent was while prototyping an experimental evm-based language which often creates small/temporary contracts. While this is not currently an economical pattern, it will likely become viable when blockchain rent makes short-lived contracts cheap. Therefore, this problem is likely to be more relevant in the future than it is now.

Your Environment

TestRPC v2.2.2 Truffle v2.0.7

axic commented 8 years ago

Just checked, this works fine in browser-solidity, which also uses ethereumjs-vm.

area commented 7 years ago

Okay, so this has been tripping me up again so I dove into it today. I was rewarded with a new revelation: I don't believe that it's nonsense data that we get back. In the case of the doubler contract above, I believe that we get returned bytes4(keccak256("double(uint256)")) zero-padded to a length of 32 i.e. the (malformed?) signature of the function it was meant to call on the new contract.

Not sure what this means, and I'm still prodding bits and pieces, but I'm really hoping this extra clue makes someone who's more familiar with testrpc and/or ethereumjs-vm go 'Ah-ha!' and will save me the bother of digging any further!

benjamincburns commented 7 years ago

There's an off chance that the work I did last week to serialize (as in the opposite of parallelize) eth_call operations might have fixed this. Will retest.

benjamincburns commented 7 years ago

Based on the comments in MetaMask/provider-engine/#166 - and on the fact that I added a checkpoint & revert, I think this is likely fixed. Haven't actually tested yet.

benjamincburns commented 7 years ago

Thanks for submitting this one, @PeterBorah... it's been a much more educational bug than I expected it to be!

Turns out that it works in the newest version of ganache-core (1.1.4), but you need to specify a higher gas limit, as the default is 90000, and the CREATE opcode by itself uses 32000 gas. You can set the gas to a higher limit by specifying it as the 2nd argument to your call function like so:

return delegator.doubleWithTempContract.call(3, { gas: 3141592 })
    .then(function(result) {
      assert.equal(result, 6);
    });

The current version of TestRPC targets ganache-core 1.1.3, but I'll do my best to get a release out which uses 1.1.4 later today. It'll very likely be version 5.0.0.

Aside, I'm actually unsure how much gas this call requires on the whole, as I'm having trouble getting the correct value out of the eth_estimateGas call. I'll chase this up with the ethereumjs-vm folks, however.

benjamincburns commented 7 years ago

Also bear in mind that (I think) the latest Solidity seems to include a nice feature which checks the gas requirement for the CREATE ahead of time and does a REVERT if the gas limit specified for the transaction/call isn't high enough. As a result, you'll get a revert runtime error message on this one if you don't set your gas to a high enough value, rather than an out of gas.