8000 Send / Transfer fails with 'invalid opcode' if destination fallback function instrumented. · Issue #106 · sc-forks/solidity-coverage · GitHub
[go: up one dir, main page]
More Web Proxy on the site http://driver.im/
Skip to content

Send / Transfer fails with 'invalid opcode' if destination fallback function instrumented. #106

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Closed
lewisbarber opened this issue Sep 10, 2017 · 13 comments · Fixed by #345
Closed

Send / Transfer fails with 'invalid opcode' if destination fallback function instrumented. #106

lewisbarber opened this issue Sep 10, 2017 · 13 comments · Fixed by #345
Assignees
@cgewecke
Labels
0.7.0 bug

Comments

@lewisbarber
Copy link
lewisbarber commented Sep 10, 2017 •
edited
Loading

So I've tried everything in the FAQ, but I'm still getting 'invalid opcode' error messages when trying to call the sendTransaction method of web3. Any idea why this might be happening?

When running the tests using only truffle it works... Perhaps I'm missing something?
@cgewecke
Copy link
Member

@lewisbarber If you could share a link to the code that's producing this error (or generate a minimal truffle project example that produces it) I'll happily take a look at it.

@lewisbarber
Copy link
Author

Hey @cgewecke, thanks!

https://github.com/sharpe-capital/sharpe-token/tree/feature/presale-bounties

This branch produces the problem:

truffle test - everything works
npm run coverage - nothing works!

@cgewecke
Copy link
Member

@lewisbarber Thanks, I'm getting these errors on the coverage run.
sharpe-coverage

Is that what you're seeing as well?

When I run the regular tests: npm test (or truffle test ) I see:

sharpe-gas

Using Node 7.10.0, on OSX.

Could you verify you're getting a clean run on your end? Or advise re: getting everything to pass so there's a baseline to compare to?

@lewisbarber
Copy link
Author
lewisbarber commented Sep 10, 2017 •
edited
Loading

So your problem with truffle test is you need to run testrpc with more gas per block (so it's more like the live network - our code does quite a bit)... The command is:

testrpc -a 10 -l 4000000000

The first error you show for the coverage is the same as what I see.

@cgewecke
Copy link
Member

@lewisbarber Thanks for reporting this.

The problem is with the transfer here. It's gas constrained by design and because the MultiSigWallet's fallback function is getting instrumented with some expensive events the call maxes out.

Tests will pass if you exclude the MultiSigWallet from the instrumentation process by adding the following to .solcover.js:

skipFiles: ['lib/MultiSigWallet.sol']

Don't know if you have a pressing audit coming up or something but that Wallet's been combed over pretty carefully so leaving it out might not matter much.

Renaming your issue to reflect the underlying problem.

@cgewecke cgewecke added the bug label Sep 11, 2017
@cgewecke cgewecke self-assigned this Sep 11, 2017
@cgewecke cgewecke changed the title 'invalid opcode' when running tests via coverage tool Send / Transfer fails with 'invalid opcode' if destination fallback function instrumented. Sep 11, 2017
@lewisbarber
Copy link
Author
lewisbarber commented Sep 11, 2017 •
edited
Loading

Ah - that makes perfect sense. I think it's acceptable for us to exclude the MultiSigWallet from the coverage report. In fact, we should probably exclude all third-party libs.

Thanks for taking a look at this so quickly, it's great to have such an important Solidity library properly supported!

@area
Copy link
Contributor
area commented Sep 11, 2017

Add this to the growing list of reasons to motivate us to move to the debug_tracetransaction approach!

@cgewecke
Copy link
Member

@area +1.

Think I will try modifying the call / transfer stipends in opFns.js and also do #92 (testrpc upgrade) today - hopefully that will fix this for the short term. . .

@cgewecke cgewecke mentioned this issue Sep 12, 2017
Merged
@cgewecke
Copy link
Member

Closing via #108. Published with 0.2.3.

@area area mentioned this issue Sep 14, 2017
Closed
This was referenced Sep 28, 2017
Closed
Merged
@cgewecke
Copy link
Member
cgewecke commented Oct 2, 2017

@lewisbarber Just a heads up - we're reverting our fix for this. I don't think that will affect you since you were going to exclude all 3rd party code in the skipFiles....

Re-opening. This will continue to happen for the time being.

@cgewecke cgewecke reopened this Oct 2, 2017
@cgewecke cgewecke added the 0.7.0 label Dec 28, 2017
@sohkai sohkai mentioned this issue Jul 24, 2018
Merged
sohkai added a commit to aragon/aragonOS that referenced this issue Jul 24, 2018
@sohkai
Fix coverage: skip some ETH transfer tests (#367)
305e83b 
Adds a utility to skip tests if they're being run in coverage.

A few of the tests relying on ETH transfers (the proxy recoverability ones) seem to fail under coverage for unknown reasons by reverting (as far as I can tell, the fallbacks aren't instrumented and it shouldn't be due to gas, but could still be related to sc-forks/solidity-coverage#106 (comment)).
@cgewecke cgewecke mentioned this issue Jul 14, 2019
Merged
@BrendanChou
Copy link
BrendanChou commented Jul 17, 2019 •
edited
Loading

@cgewecke is this supposed to be fixed now? I'm confused about why I'm still seeing errors for it in 0.6.0. Specifically when unwrapping WETH, the WETH contracts sends ETH to an empty fallback function that reverts when under coverage

@cgewecke
Copy link
Member
cgewecke commented Jul 17, 2019 •
edited
Loading

@BrendanChou Yes it looks like it's fixed on 0.6.2 (testrpc-sc v6.4.5-sc.3). If you're still seeing it with 0.6.2 could you link to a repro when you have a chance?

We have a passing unit test for the send/transfer case here. Its relevant solidity source and js test are here:

@BrendanChou
Copy link

Awesome, 0.6.2 fixed it, thank you!

@elenadimitrova elenadimitrova mentioned this issue Jun 10, 2020
Open
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@cgewecke cgewecke

Labels
0.7.0 bug
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

Upgrade testrpc-sc 6.4.5-sc.3 sc-forks/solidity-coverage
4 participants
@area @BrendanChou @lewisbarber
0