PolymathNetwork / polymath.js

10 stars 6 forks source link

Dry run failing from a valid blockchain transaction #6

Closed davekaj closed 6 years ago

davekaj commented 6 years ago

From the front end using polymath.js, I get a dry run failure error as follows: Transaction dry run failed: Invalid JSON RPC response: {"id":144,"jsonrpc":"2.0"} (the ID always changes).

I believe it has to do with a dry run being deemed not valid, when really it is.

How to reproduce: On the whitelist branch go through the steps of making a token, and then submit some investors to the blockchain, either through a CSV or Add user button. Give them real time stamps for _to and _from times.

Then Try to remove the investor by passing it a 0 timestamp/date. When you do this, using polymathjs function call, it will fail on the dry run.

Trying directly through truffle console works, so I believe I have narrowed it down to this block of code below from polymathjs :

  async _tx (method: Object, value?: BigNumber): Promise<Web3Receipt> {
    const preParams = {
      from: this.account,
      value: value ? this._toWei(value) : undefined
    }
    const params = {
      ...preParams,
      gas: Math.floor(await method.estimateGas(preParams) * 1.05) // TODO @bshevchenko: https://github.com/PolymathNetwork/polymath.js/issues/4
    }

    // dry run
    try {
      const okCode = this._isBoolOutput(method._method.name)
      const dryResult = await method.call(params)
      if (okCode && dryResult !== okCode) {
        throw new Error(`Expected ${okCode === true ? 'true' : okCode}, but received ${dryResult}`)
      }
    } catch (e) {
      throw new Error(`Transaction dry run failed: ${e.message}`)
    }

    const receipt = await method.send(params, (error, hash) => {
      if (!error) {
        Contract._params.txHashCallback(hash)
      }
    })
    Contract._params.txEndCallback(receipt)

    if (receipt.status === '0x0') {
      throw new Error('Transaction failed')
    }

    return receipt
  }

Speficially I think this line is returning the weird error : const dryResult = await method.call(params)

bshevchenko commented 6 years ago

@davekaj you tried it on Ropsten or ganache? Because I see different errors on ganache. When I try set existent whitelist entry to zero I see: Error: Transaction dry run failed: Returned error: base fee exceeds gas limit When I try to increase tx gas amount on 25% I see: Error: Transaction dry run failed: Returned error: VM Exception while processing transaction: out of gas But it works when I increase it on 100%. So it's related to #4

bshevchenko commented 6 years ago

@davekaj please check with polymathjs@0.4.1 and answer me about network that you use – ropsten or ganache? Error message is necessary for possible #4 solution

davekaj commented 6 years ago

I used ganache - i installed polymahtjs 0.4.1, trying it now but getting other errors, will update when i am able to reproduce

davekaj commented 6 years ago

The new update to polymathjs@0.4.1 does fix this error. However it is weird that in my environment, I still get the Transaction dry run failed: Invalid JSON RPC response: {"id":144,"jsonrpc":"2.0"} error, and @bshevchenko gets the more accurate errors about not having enough gas. Maybe it has to do with my environment, which is as follows:

Ganache CLI v6.1.0 (ganache-core: 2.1.0) Chrome Version 65.0.3325.181 Branch dave-dev of polymath-issuer (updated Wednesday april 10th, with master branch pulled in on april 10th) Truffle version 4.1.3^ (this is directly from polymath-issuer package.json, so it shouldnt matter what your global truffle version is)

We shall keep this open for a while, to see if we can find a solution, and allow others to see it and possibly contribute

davekaj commented 6 years ago

https://ethereum.stackexchange.com/questions/45508/calling-a-function-to-zero-out-struct-data-in-a-mapping-costing-more-gas-than-ot

opened a question on ethereum stack exchange

davekaj commented 6 years ago

It appears to be a problem with eth_estimateGas, from ganache. this issue in geth https://github.com/ethereum/go-ethereum/issues/3719 shows that it actually should work with a real geth node, but apparently it doesnt work with testrpc/ganache.

so i chased down ganache-core issues, which lead to it being clear that binary tree isnt implemented in ganache : https://github.com/trufflesuite/ganache-core/pull/75

Clicking through, you get to https://github.com/trufflesuite/ganache-core/issues/26 . Here you have a suggested fix, by replacing this line in ganache core: https://github.com/trufflesuite/ganache-core/blob/c7a2f69cad6bd8d73a67c300e8eb9816b44d694c/lib/statemanager.js#L585

with return callback(null, to.hex(results.gasUsed.iadd(results.vm.gasRefund)))

The binary tree implementation for ganache is blocked right now. It appears it will work for both metamask and geth, so the only place we need to be concerned with is when it fails with ganache. Our options are to implement the fix above, or have some other round about way for providing more gas to this function specifically.

Link to metamask PR for binary search: Also it appears metamask is using the binary search https://github.com/MetaMask/provider-engine/pull/119/files

pabloruiz55 commented 6 years ago

FYI this is also failing on Ropsten. When trying to launch the STO I will get a "gas required exceeds..." error pop without Metamask never popping up.

davekaj commented 6 years ago

I did not look in depth if metamask actually fixed the issue, just read the title of the PR. will have to look deeper

bshevchenko commented 6 years ago

@davekaj any updates on that?

davekaj commented 6 years ago

I looked at metamasks current master branch, they have the binary tree working https://github.com/MetaMask/provider-engine/blob/16df4415742b7212e0d5d58394452f39f682e958/subproviders/vm.js#L66

So this shouldnt result in an issue when we interact with the dapp with metamask

bshevchenko commented 6 years ago

Everything works fine now on Ropsten, Rinkeby and Ganache