Add support and tests for ejsvm allowUnlimitedContractSize #126
Conversation
|
|
||
| // Thanks solc. At least this works! | ||
| // This removes solc's overzealous uncaughtException event handler. | ||
| process.removeAllListeners("uncaughtException"); |
There was a problem hiding this comment.
These tests likely belong in vm.js
There was a problem hiding this comment.
I disagree. I don't think we should bombard a bunch of "junk" into one file. I would much rather support a folder-based structure.
For example we have vm.js and unlimited_contract_size.js. I think this would be better as vm/revert.js and vm/unlimited_contract_size.js rather than putting it all in vm.js and causing chaos for maintenance.
There was a problem hiding this comment.
From @benjamincburns on Slack:
if you want to reorganize for folder structure for tests that's fine, but they should share test setup/teardown
that's the key benefit of grouping like tests
honestly I think some test engineering is warranted in ganache-cli for setup/teardown
for now feel free to merge as-is as well and we can discuss that later
or fold it into The Refactoring
I'm going to go ahead and merge this as is; we'll discuss at a later time on how testing is structured
This PR is a clean replacement for #79. #79 got messed up with some issue with rebasing, so I recreated the effective changes in a new branch/commit. You can see #79's description below:
I'm creating a real-time Solidity debugger runtime for an Augur bounty.
To do this sanely, compiler optimization needs to be turned off for the developer to be follow a logical stepping process; otherwise it's possible for the compiler to optimize things away and cause unexpected behavior from a debugging perspective.
Unfortunately, turning off optimization can mean very large compiled contracts. EIP 170 prevents contracts with more than 0x6000 bytes (or 24KB) from being created. The rationale for EIP 170 (as I interpreted it) was really to address scaling from a global perspective, however I can see it acceptable to allow users to ignore EIP 170, specifically for debugging purposes.
This PR adds the option for ignoring EIP 170. The option will not do anything until the associated ethereumjs-vm PR (ethereumjs/ethereumjs-monorepo#282) has been merged.
At first I figured ganache-core would always want the EIP 170 ignored, but after learning more about Ganache it seems that applications will use ganache-core with a "production blockchain" mindset as well; therefore I am using the existing debug flag to indicate whether or not this flag should be ignored.
I'm open to the idea to adding an additional external option for ignoring EIP 170 if that would make more sense.