6.4.2: `TypeError: Cannot read property '0' of undefined`

tyler17 commented 5 years ago

Expected Behavior

ganache-cli not crashing

Current Behavior

After upgrading to v6.4.2 (from v6.2.5), the testchain will randomly crash in the middle of our tests with

eth_blockNumber
eth_blockNumber
eth_blockNumber
TypeError: Cannot read property '0' of undefined
    at e (/Users/Tyler/dai.js/testchain/node_modules/ganache-cli/build/ganache-core.node.cli.js:69:5393)
    at u (/Users/Tyler/dai.js/testchain/node_modules/ganache-cli/build/ganache-core.node.cli.js:69:5429)
    at u (/Users/Tyler/dai.js/testchain/node_modules/ganache-cli/build/ganache-core.node.cli.js:69:5955)
    at u (/Users/Tyler/dai.js/testchain/node_modules/ganache-cli/build/ganache-core.node.cli.js:69:5770)
    at /Users/Tyler/dai.js/testchain/node_modules/ganache-cli/build/ganache-core.node.cli.js:69:4169
    at /Users/Tyler/dai.js/testchain/node_modules/ganache-cli/build/ganache-core.node.cli.js:61:1861552
    at /Users/Tyler/dai.js/testchain/node_modules/ganache-cli/build/ganache-core.node.cli.js:2:26124
    at i (/Users/Tyler/dai.js/testchain/node_modules/ganache-cli/build/ganache-core.node.cli.js:2:41179)

Steps to Reproduce

checkout branch sdk-688 here: https://github.com/makerdao/dai.js/tree/sdk-688 follow steps under Developing in Readme run tests with yarn test

Your Environment

Version used: 6.4.2
NodeJS Version: v10.13.0
Operating System and version: macOS High Sierra, version 10.13.4 Beta
Link to your project or repro gist: https://github.com/makerdao/dai.js/tree/sdk-688
Commit hash to use with above link for reproduction: 7a246d18b91f259bb9e3f9ec1a064fba96ab9619

ndeshev commented 5 years ago

I can confirm that this problem exists, in my opinion, this is a critical issue as it concerns large amount of use cases.

It also makes using ganache-cli with MetaMask impossible, as it crashes every time a transaction to a contract function has to be signed, so I had to switch back to version 6.4.1.

The problem is related to the new "gas exactimation" methods used in versions > 6.4.1, I believe.

davidmurdoch commented 5 years ago

Thanks @tyler17 for the report. I'll try to get a hotfix release out for this today!

davidmurdoch commented 5 years ago

@ndeshev, what leads you to believe this crash is gas estimation related? Can you add some log output here? (starting ganache-cli with the --verbose flag would be extra helpful)

ndeshev commented 5 years ago

David, yes, I tested it with the verbose option on. Also the contracts passed about 200 unit tests in Truffle where I never estimate gas usage and the only time the ganache-cli breaks with TypeError: Cannot read property '0' of undefined error is when a link to a contract function is clicked and the MetaMask transaction sign window has to pop up with the gas price and the total cost of the transaction.

The estimated gas for the transaction that the MetaMask is displaying is almost the maximum I set for allowed for block gas limit. If I leave the MetaMask transaction window open and restart the ganache-cli it sends it successfully.

Also, the problem did not exist in versions lower than 6.4.2 which is another thing that led me to think is gas estimation problem related as this was one of the few changes in versions 6.4.2 and 6.4.3.

The estimated gas for the transactions that is displayed in the logs below are incorrect - it is almost the maximum allowed for the block, as mentioned above.

I did some more testing, there are functions that do not break the ganache client when MetaMask is invoked, but they are very simple short ones, on calls to more complicated functions the problem persists.

Update - More testing and information related to use cases with web3.js: After executing only the web3.js gas estimation method contract.methods.myMethod().estimateGas() on a function call (that crashes ganache when called via MetaMask), the ganache-cli does NOT break, but it estimates super incorrect gas required for the call - more precisely 46k for a function transaction that requires 650k, meaning that the gas is very underestimated, as opposed to the MetaMask case above.

However, when I tried to run the same web3.js .estimateGas() method on a ganache-cli prior to v.6.4.2 it doesn't work at all throwing the following error:

RPC Error: Internal JSON-RPC error. Object { code: -32603, message: "Internal JSON-RPC error." }

In summary, that might be a tricky one...

OS: Windows 10, web3.js version: 1.00-beta.52 | Node - 10.14.1 | ganache-cli version used for the logs is 6.4.3 but when I tried 6.4.2 before a few days the outcome was the same.

Log 1:

eth_blockNumber { "id": 1333707413168176, "jsonrpc": "2.0", "params": [], "method": "eth_blockNumber" } < { < "id": 5050169228901374, < "jsonrpc": "2.0", < "result": "0x1e43" < } < { < "id": 1333707413168176, < "jsonrpc": "2.0", < "result": "0x1e43" < } eth_estimateGas { "id": 4525324431062, "jsonrpc": "2.0", "params": [ { "from": "0x2b235b23be383bfd75d87f865f6fb74536072f57", "to": "0x81e86b03500da6377478272f2dcde68e33ec360a", "value": "0x3bf3b91c95b00000", "data": "0xbaaa1ea8000000000000000000000000000000000000000000000000000000000000000500000000000000000000000018c6a286552899dc9d74b3dbd3dc26bf80d277a1", "gasPrice": "0x4a817c800", "gas": "0x657890" } ], "method": "eth_estimateGas" } TypeError: Cannot read property '0' of undefined at e (C:\Users\ndesh\AppData\Roaming\npm\node_modules\ganache-cli\build\ganache-core.node.cli.js:69:5393) at u (C:\Users\ndesh\AppData\Roaming\npm\node_modules\ganache-cli\build\ganache-core.node.cli.js:69:5429) at u (C:\Users\ndesh\AppData\Roaming\npm\node_modules\ganache-cli\build\ganache-core.node.cli.js:69:5955) at u (C:\Users\ndesh\AppData\Roaming\npm\node_modules\ganache-cli\build\ganache-core.node.cli.js:69:5770) at C:\Users\ndesh\AppData\Roaming\npm\node_modules\ganache-cli\build\ganache-core.node.cli.js:69:4169 at C:\Users\ndesh\AppData\Roaming\npm\node_modules\ganache-cli\build\ganache-core.node.cli.js:61:1853218 at C:\Users\ndesh\AppData\Roaming\npm\node_modules\ganache-cli\build\ganache-core.node.cli.js:2:26124 at i (C:\Users\ndesh\AppData\Roaming\npm\node_modules\ganache-cli\build\ganache-core.node.cli.js:2:41179) at C:\Users\ndesh\AppData\Roaming\npm\node_modules\ganache-cli\build\ganache-core.node.cli.js:61:1210468 at C:\Users\ndesh\AppData\Roaming\npm\node_modules\ganache-cli\build\ganache-core.node.cli.js:2:105466 at C:\Users\ndesh\AppData\Roaming\npm\node_modules\ganache-cli\build\ganache-core.node.cli.js:32:392 at c (C:\Users\ndesh\AppData\Roaming\npm\node_modules\ganache-cli\build\ganache-core.node.cli.js:32:5407) at C:\Users\ndesh\AppData\Roaming\npm\node_modules\ganache-cli\build\ganache-core.node.cli.js:32:317 at C:\Users\ndesh\AppData\Roaming\npm\node_modules\ganache-cli\build\ganache-core.node.cli.js:61:1871644 at C:\Users\ndesh\AppData\Roaming\npm\node_modules\ganache-cli\build\ganache-core.node.cli.js:2:23237 at o (C:\Users\ndesh\AppData\Roaming\npm\node_modules\ganache-cli\build\ganache-core.node.cli.js:2:26646) at C:\Users\ndesh\AppData\Roaming\npm\node_modules\ganache-cli\build\ganache-core.node.cli.js:2:26124 at C:\Users\ndesh\AppData\Roaming\npm\node_modules\ganache-cli\build\ganache-core.node.cli.js:61:1864681 at C:\Users\ndesh\AppData\Roaming\npm\node_modules\ganache-cli\build\ganache-core.node.cli.js:61:1862544 at C:\Users\ndesh\AppData\Roaming\npm\node_modules\ganache-cli\build\ganache-core.node.cli.js:61:1889757 at C:\Users\ndesh\AppData\Roaming\npm\node_modules\ganache-cli\build\ganache-core.node.cli.js:2:23237 at o (C:\Users\ndesh\AppData\Roaming\npm\node_modules\ganache-cli\build\ganache-core.node.cli.js:2:26646)

Sometimes ganache executes one or few more JSON RPC call before crashing, everything else is the same, the gas cost is again at the maximum allowed block gas limit.

eth_estimateGas { "id": 4525324431077, "jsonrpc": "2.0", "params": [ { "from": "0x8f9e2539f358b13af290d323f7b9220983745a98", "to": "0x81e86b03500da6377478272f2dcde68e33ec360a", "value": "0x1df9dc8e4ad80000", "data": "0xbaaa1ea8000000000000000000000000000000000000000000000000000000000000000500000000000000000000000018c6a286552899dc9d74b3dbd3dc26bf80d277a1", "gasPrice": "0x4a817c800", "gas": "0x657890" } ], "method": "eth_estimateGas" } eth_blockNumber { "id": 1333707413168638, "jsonrpc": "2.0", "params": [], "method": "eth_blockNumber" } TypeError: Cannot read property '0' of undefined at e (C:\Users\ndesh\AppData\Roaming\npm\node_modules\ganache-cli\build\ganache-core.node.cli.js:69:5393) at u (C:\Users\ndesh\AppData\Roaming\npm\node_modules\ganache-cli\build\ganache-core.node.cli.js:69:5429) at u (C:\Users\ndesh\AppData\Roaming\npm\node_modules\ganache-cli\build\ganache-core.node.cli.js:69:5955)..........

davidmurdoch commented 5 years ago

@ndeshev we found a potential fix, but can't pinpoint the contract that causes the condition that triggers the bug. Can you share with us the contract and method(s) that are causing the crash?

davidmurdoch commented 5 years ago

For our internal reference, here is the potential fix: https://github.com/trufflesuite/ganache-core/commit/b74050bd5505b92fcc123affb52108c4c5b3004b

ndeshev commented 5 years ago

Hey there David, I apologize for the delayed response. Unfortunately, at this stage I'm not able to provide the full contracts code, I will try to help by describing more of my observations using and testing the 6.4.3 version with the latest MM and web3.js (1.00-beta.52) for multi-contract Dapp, during the last few days.

If default gas or the gas option for the contract call is set in web3 (like myContract.methods.myMethod([param1[, param2[, ...]]]).send( { from: '0x0', gas: 1234 })) the ganache does not break, probably because MetaMask gets the the gas values directly and never requests eth_estimateGas for transaction.
In my comment above I said that the only functions that did not break the client are the simple ones - after more tests that turned out to not be the case as some of the bigger functions (400k+ gas required) do not break it and also some smaller ones do (as the one included below).
For calls that do NOT crash the ganache-cli: The higher the gas limit for the block is set - the higher the estimated gas required for the call is, for example, if the block gas limit is 8M the estimated gas for the call is 100k, if the block gas limit is 80M the estimation is 800K (example values) - very weird behavior...
Even if the ganache-cli does not break during the function triggering, the gas estimates returned to the MM plugin are very very overestimated, sometimes double and even triple the real gas requirement

All calls are going through multiple contracts, meaning: call to an entry contract - > that calls a logic contract -> that calls storage contract, etc. Solidity compiler version is 0.5.7, the Ethereum fork used is St. Petersburg, all ganche-cli options used:

ganache-cli -a 50 -i 12 -b 14 -e 12000 -l 8000000 --deterministic --mnemonic "inform gentle manage deputy dismiss crumble want clever cheap purple rain prince" --db "D:/TestRPC/X"

The code of one of the most simple function calls that crash the client: Overview: Calling a function from an entry point contract that is responsible for withdrawing a payment under certain conditions. That entry contract then calls another contract to execute the logic required.

// In Main (router) contract
function withdrawPayment(uint256 entry) external nonReentrant {
    payments.withdrawPayment(msg.sender, entry);
}

// Payment contract
function withdrawPayment(address payable payee, uint256 entry) 
    external
    onlyMain 
{           
    require(block.number > storage.eventPeriod(entry) "Not allowed"); 
    uint256 amount = storage.getPayment(payee, entry);
    require(amount > 0, "No funds to withdraw");
    storage.setPayment(payee,  0);
    // Transfer the funds from the escrow contract
    escrow.withdraw(payee, amount);
    // Emit an event via another contract
    logEmitter.emitPaymentWithdrawed(payee, entry, amount);
}

The log:


eth_estimateGas
   > {
   >   "id": 2364203582590,
   >   "jsonrpc": "2.0",
   >   "params": [
   >     {
   >       "from": "0x57167f9651b39990d7cb07bc2b7d1de2298d9560",
   >       "to": "0x81e86b03500da6377478272f2dcde68e33ec360a",
   >       "data": "0x4f72a2970000000000000000000000000000000000000000000000000000000000000000",
   >       "gasPrice": "0x4a817c800",
   >       "value": "0x0",
   >       "gas": "0x73f780"
   >     }
   >   ],
   >   "method": "eth_estimateGas"
   > }
eth_blockNumber
   > {
   >   "id": 7189048665149203,
   >   "jsonrpc": "2.0",
   >   "params": [],
   >   "method": "eth_blockNumber"
   > }
TypeError: Cannot read property '0' of undefined
    at e (C:\Users\ndesh\AppData\Roaming\npm\node_modules\ganache-cli\build\ganache-core.node.cli.js:69:5393)
    at u (C:\Users\ndesh\AppData\Roaming\npm\node_modules\ganache-cli\build\ganache-core.node.cli.js:69:5429)
    at u (C:\Users\ndesh\AppData\Roaming\npm\node_modules\ganache-cli\build\ganache-core.node.cli.js:69:5955)
    at u (C:\Users\ndesh\AppData\Roaming\npm\node_modules\ganache-cli\build\ganache-core.node.cli.js:69:5770)
    at C:\Users\ndesh\AppData\Roaming\npm\node_modules\ganache-cli\build\ganache-core.node.cli.js:69:4169

Another log on another execution of the same function:

 <     "result": []
 <   }
eth_estimateGas
   > {
   >   "id": 2364203582600,
   >   "jsonrpc": "2.0",
   >   "params": [
   >     {
   >       "from": "0x892959323c5bb663b2e3e5951513dfa6c570c5c5",
   >       "to": "0x81e86b03500da6377478272f2dcde68e33ec360a",
   >       "data": "0x4f72a2970000000000000000000000000000000000000000000000000000000000000000",
   >       "gasPrice": "0x4a817c800",
   >       "value": "0x0",
   >       "gas": "0x73f780"
   >     }
   >   ],
   >   "method": "eth_estimateGas"
   > }
eth_getTransactionCount
   > {
   >   "id": 2200776223303145,
   >   "jsonrpc": "2.0",
   >   "params": [
   >     "0x892959323c5bb663b2e3e5951513dfa6c570c5c5",
   >     "0x142c"
   >   ],
   >   "method": "eth_getTransactionCount"
   > }
eth_getBalance
   > {
   >   "id": 7943234958535992,
   >   "jsonrpc": "2.0",
   >   "params": [
   >     "0x09769374278d04dc09a51d3f3c1955f21bac9da5",
   >     "0x142c"
   >   ],
   >   "method": "eth_getBalance"
   > }
eth_getBalance
   > {
   >   "id": 7943234958535993,
   >   "jsonrpc": "2.0",
   >   "params": [
   >     "0x18c6a286552899dc9d74b3dbd3dc26bf80d277a1",
   >     "0x142c"
   >   ],
   >   "method": "eth_getBalance"
   > }
eth_getBalance
   > {
   >   "id": 7943234958535994,
   >   "jsonrpc": "2.0",
   >   "params": [
   >     "0x8f9e2539f358b13af290d323f7b9220983745a98",
   >     "0x142c"
   >   ],
   >   "method": "eth_getBalance"
   > }
eth_getBalance
   > {
   >   "id": 7943234958535995,
   >   "jsonrpc": "2.0",
   >   "params": [
   >     "0xd70a0ffc6bd1f295b75a113928d5f09adae6990f",
   >     "0x142c"
   >   ],
   >   "method": "eth_getBalance"
   > }
eth_blockNumber
   > {
   >   "id": 7189048665150313,
   >   "jsonrpc": "2.0",
   >   "params": [],
   >   "method": "eth_blockNumber"
   > }
TypeError: Cannot read property '0' of undefined
    at e (C:\Users\ndesh\AppData\Roaming\npm\node_modules\ganache-cli\build\ganache-core.node.cli.js:69:5393)
    at u (C:\Users\ndesh\AppData\Roaming\npm\node_modules\ganache-cli\build\ganache-core.node.cli.js:69:5429)
    at u (C:\Users\ndesh\AppData\Roaming\npm\node_modules\ganache-cli\build\ganache-core.node.cli.js:69:5955)
    at u (C:\Users\ndesh\AppData\Roaming\npm\node_modules\ganache-cli\build\ganache-core.node.cli.js:69:5770)
    at C:\Users\ndesh\AppData\Roaming\npm\node_modules\ganache-cli\build\ganache-core.node.cli.js:69:4169
    at C:\Users\ndesh\AppData\Roaming\npm\node_modules\ganache-cli\build\ganache-core.node.cli.js:61:1853218
    at C:\Users\ndesh\AppData\Roaming\npm\node_modules\ganache-cli\build\ganache-core.node.cli.js:2:26124
    at i (C:\Users\ndesh\AppData\Roaming\npm\node_modules\ganache-cli\build\ganache-core.node.cli.js:2:41179)
    at C:\Users\ndesh\AppData\Roaming\npm\node_modules\ganache-cli\build\ganache-core.node.cli.js:61:1210468

I will test the potential fix in a few days

tyler17 commented 5 years ago

@davidmurdoch Awesome, just wanted to confirm that I tried using that fix/gas-est-crash branch locally and I'm not getting crashes anymore

ndeshev commented 5 years ago

Confirming that the potential fix resolves the crashes during UI testing with MM & web3.js. The other gas estimation problems described in https://github.com/trufflesuite/ganache-core/issues/417#issuecomment-484739359 are the same. Mainly: The gas estimates are way overestimated, 2x, 3x, 4x the actual amount required. Also, the block gas limit setting has a very strange increasing/decreasing effect on them.

davidmurdoch commented 5 years ago

Thanks @ndeshev for following up. I wasn't able to reproduce the crash with the contract examples you provided since they are incomplete contracts. I'm hesitant to release this patch without a test to ensure we don't regress in the future. Is there any chance you can provide us with a complete reproduction contract.

The overestimate is definitely a separate bug. @nicholasjpaterno will likely be looking into this issue when he wraps up some of his current tasks. Changing the blockGasLimit having an effect on estimation isn't too surprising (probably still a bug if we are returning fluctuating gas estimates), as many contract functions do require additional gas the more gas you provide (we use the blockGasLimit as the default gas amount in our gas estimate calculations). See EIP-150 (specifically the EIP-114 proposal it implements) for details.

There is one edge case we do know about and don't try to handle: when a contract checks its gasLeft and behaves differently depending on this value. A contrived example being:

  if (gasLeft % 2 == 0) {
     expensive();  
  } else {
    cheap();
  }

A function like the above would cause gas estimations to vary wildly depending on the amount of gas initially supplied to the transaction.

Just to reiterate: the amount of gas required is often greater than the amount of gas used. Any chance you could provide a minimal reproduction of a fluctuating and wildly high gas estimate contracts?

tyler17 commented 5 years ago

@davidmurdoch Are you still unable to reproduce the crash? I'm finding that it crashes with some (but not all) calls made to DS Proxy's execute function, maybe the delegatecall is causing the issue?

davidmurdoch commented 5 years ago

Thanks @tyler17! Yes, we're still looking for a minimal reproduction of the issue. This line looks like great candidate to produce an edge case in opcode ordering we don't handle: https://github.com/dapphub/ds-proxy/blob/379f5e2fc0a6ed5a7a96d3f211cc5ed8761baf00/src/proxy.sol#L64

@nicholasjpaterno :point_up:

sirpy commented 5 years ago

@davidmurdoch This still happens in beta 2.5.6. on eth_estimateGas It happens when trying to execute an erc 677 transferAndCall. If I set the gas to fixed amount (200000) it works fine otherwise ganache crashes with the above error right after eth_estimateGas

The client code is in this method: https://github.com/GoodDollar/GoodDAPP/blob/33b78201078fe72604d23a5a8a867ca6fd245e31/src/lib/wallet/GoodWallet.js#L402

The contracts code is here: ERC677 - https://github.com/GoodDollar/GoodContracts/blob/47079ad5005526f204760b9c1b47fcd9ad04515d/contracts/GoodDollar.sol#L94

The called method after transfer https://github.com/GoodDollar/GoodContracts/blob/47079ad5005526f204760b9c1b47fcd9ad04515d/contracts/OneTimePaymentLinks.sol#L50

trufflesuite / ganache