Upgrade testrpc-sc

[cgewecke]

There is growing drift between ganache-cli and testrpc-sc.

For example, since 6.1.3, reason string functionality has been added and the client has begun returning false when a view function fails a require gate (instead of throwing a revert error). . .

[area]

We'd very much like the reason string functionality in Colony, so I'm taking a crack at this, despite not having done it since it became so complicated! Everything in the upgrading testrpc-sc file seems to be right, apart from the fact that I can't see how ganache-core-sc depends on provider-engine-sc. Is that no longer the case?

EDIT: The instructions have a push after a rebase, but that'll require a --force-with-lease, right?

EDIT2: I'm putting coverage-20180803 branches on everything, rather than touching the coverage branches, so I can test everything out without irreversibly ruining something 😛

[cgewecke]

Oh thanks! Is it clear what the changes at VM are? I made some last falll after byzantium that short-circuit the limits on contract size from EIP 170. Newer ethereumjs-vm has a flag that makes this optional . . .

I can't see how ganache-core-sc depends on provider-engine-sc. Is that no longer the case?

Yes, no longer the case

The instructions have a push after a rebase, but that'll require a --force-with-lease, right?

Yes. Actually I've never heard of the -with-lease, have to look that up. But force. You could do this however you want though . . .

[area]

So the relevant branches are coverage-20180803 on everything apart from solidity-coverage, where the branch is 20180803. I'm running against our (Colony's) test suite with the 20180803 branch, but it looks good so far.

[cgewecke]

Ok great!

[area]

Even though it seems to work fine for us, I can't get the build to work for some reason 😢

[cgewecke]

@area Ah ok - I'll take a look this afternoon....

[cgewecke]

@area Was it complaining about not being able to find ethereumjs-vm? It builds with yarn (which makes absolutely no sense!)

Thanks for doing this. Think 5.1.7 has the changes unless I messed it up.

[ajhodges]

Hey @cgewecke, I was hoping that due to the discussion in this thread maybe testrpc-sc had been updated to handle reason string functionality in reverts, but as far as I can tell, solidity-coverage v0.5.11 still does not handle revert reason strings. It looks like this was added in ganache-core v2.2.0: https://github.com/trufflesuite/ganache-core/releases/v2.2.0 which was released after the latest release of testrpc-sc.

(I am trying to use solidity-coverage in conjunction with https://github.com/rkalis/truffle-assertions, which tests the reason message included in the revert error message).

[remon-nashid]

I can confirm @ajhodges's findings. Latest solidity-coverage still doesn't handle revert reason as shown here OpenZeppelin/openzeppelin-contracts#1540