Error when payable contract method deploys another contract and dispatches event.

ericwooley commented 6 years ago

When running ganache, deploying a contract from a contract method errors.

Expected Behavior

Successful transaction

Current Behavior

Error: VM Exception while processing transaction: revert

Possible Solution

:( only way to proceed ATM is geth private network as far as I can tell.

Steps to Reproduce (for bugs)

clone https://github.com/ericwooley/EtherAds/tree/03f912513c474bb1a370147fde24b3e7b6a89507/packages/ad-factory
open in remix with remixd
select 0.4.18+commit.9cf6e910.Emscripten.clang for solidity version
Select the first account and copy the address.
Use the copied address as the the string input to the constructor for AdBackLogFactory (contracts/AdBackLogFactory.sol)
Deploy AdBackLogFactory using metamask connected to ganache Beta 1.1.0-beta.1
donationAddress should be your address
use the deployAd method with the arguments 500, true

You should see the following the remix console.

transact to AdLogFactory.deployAd errored: Error: Error: [ethjs-rpc] rpc error with payload {"id":6889124706343,"jsonrpc":"2.0","params":["0xf8ab01843b9aca0083082565948cdaf0cd259887258bc13a92c0a6da92698644c080b844a5ffbcf000000000000000000000000000000000000000000000000000000000000001f40000000000000000000000000000000000000000000000000000000000000001822d46a0dc74b9bce3696f711b00c01df939b9bb269b00047c92450584447074147f41eea07d3132061e5cbd83cdf590e21a4f3423ec4e49c025add7b65db40368deaf17ab"],"method":"eth_sendRawTransaction"} Error: VM Exception while processing transaction: revert
    at TransactionStateManger.<anonymous> (chrome-extension://nkbihfbeogaeaoehlefnkodbefgpgknn/scripts/background.js:2216:39)
    at TransactionStateManger.g (chrome-extension://nkbihfbeogaeaoehlefnkodbefgpgknn/scripts/background.js:69757:16)
    at TransactionStateManger.EventEmitter.emit (chrome-extension://nkbihfbeogaeaoehlefnkodbefgpgknn/scripts/background.js:69673:17)
    at TransactionStateManger._setTxStatus (chrome-extension://nkbihfbeogaeaoehlefnkodbefgpgknn/scripts/background.js:5488:14)
    at TransactionStateManger.setTxStatusFailed (chrome-extension://nkbihfbeogaeaoehlefnkodbefgpgknn/scripts/background.js:5448:12)
    at TransactionController._callee6$ (chrome-extension://nkbihfbeogaeaoehlefnkodbefgpgknn/scripts/background.js:2479:37)
    at tryCatch (chrome-extension://nkbihfbeogaeaoehlefnkodbefgpgknn/scripts/background.js:20399:40)
    at Generator.invoke [as _invoke] (chrome-extension://nkbihfbeogaeaoehlefnkodbefgpgknn/scripts/background.js:20633:22)
    at Generator.prototype.(anonymous function) [as throw] (chrome-extension://nkbihfbeogaeaoehlefnkodbefgpgknn/scripts/background.js:20451:21)
    at step (chrome-extension://nkbihfbeogaeaoehlefnkodbefgpgknn/scripts/background.js:18078:30)
    at chrome-extension://nkbihfbeogaeaoehlefnkodbefgpgknn/scripts/background.js:18091:13
    at <anonymous>

Running the same steps with a geth private network success. remix-scenarios.zip

Context

Contract Factory pattern, for deploying a contract from another contract method, with an optional donation.

Your Environment

Version used: Beta 1.1.0-beta.1
Environment name and version (e.g. PHP 5.4 on nginx 1.9.1): Solidity 0.4.18+commit.9cf6e910.Emscripten.clang
Operating System and version: OSX High Sierra
Link to your project: https://github.com/ericwooley/EtherAds/tree/03f912513c474bb1a370147fde24b3e7b6a89507/packages/ad-factory

benjamincburns commented 6 years ago

Hi @ericwooley - I'm guessing that if you increase your gas limit (add the gas parameter to the contract deploy operation) this won't fail anymore.

If this still fails, then my suspicion is that it's caused by the same problem causing this issue, which we believe to be an ethereumjs-vm bug, at the moment.

That said, even if you can make it succeed by increasing the gas limit, if you're running the exact same request on geth and/or parity and it succeeds, but then it fails on ganache, this is likely still a bug.

ericwooley commented 6 years ago

@benjamincburns You are correct, increasing the gas allowed it to go through. Is there anything the can be done to allow the gas to be estimated correctly?

When It fails, i am just allowing meta mask to estimate the gas.

The interesting part is that in the receipt, it says that the transaction cost was the same that metamask was estimating.

I have to give it almost double the amount of gas for it to be successful.

ericwooley commented 6 years ago

@benjamincburns In regards to the ethereumjs-vm bug, I ran through it with the remix version of the ethereumjs-vm runner, and it was successful. I don't know what version either project uses though.

Although, remix doesn't estimate gas, it sets it at 3000000 by default

ericwooley commented 6 years ago

I played around with remix ethereumjs-vm, and it does succeed with manual gas, that is very very close to what metamask is estimating.

benjamincburns commented 6 years ago

In regards to the ethereumjs-vm bug, I ran through it with the remix version of the ethereumjs-vm runner, and it was successful

@ericwooley thanks! that's really encouraging. I know we're at least one release behind, and I've been meaning to update. I'll give that a go today or tomorrow and report back.

mikeseese commented 6 years ago

@ericwooley we're addressing gas estimation issues in https://github.com/trufflesuite/ganache-core/issues/26 which should give a high water mark estimation. In that issue, there will be a link at some point to another issue where we'll work more accurate estimation using a VM step handler. I'm going to close this issue in favor of https://github.com/trufflesuite/ganache-core/issues/26

Thanks!

trufflesuite / ganache-ui