Skip to content
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.

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

vm.copy() doesn't preserve all options, like allowUnlimitedContractSize. #2017

Closed
davidmurdoch opened this issue Jul 5, 2022 · 2 comments · Fixed by #2027
Closed

vm.copy() doesn't preserve all options, like allowUnlimitedContractSize. #2017

davidmurdoch opened this issue Jul 5, 2022 · 2 comments · Fixed by #2027
Assignees

Comments

@davidmurdoch
Copy link
Contributor

I expected initial options to be preserved when calling vm.copy(), but this does not seem to be the case:

const assert = require("assert");
const VM = require("@ethereumjs/vm").default;

const vm = new VM({
  allowUnlimitedContractSize: true
});
const vmCopy = vm.copy();
assert.strictEqual(vm._allowUnlimitedContractSize, true); // passes
assert.strictEqual(vmCopy._allowUnlimitedContractSize, true); // fails
@holgerd77
Copy link
Member

@davidmurdoch Hi there, yes that's a bug, thanks for reporting!

I guess this is also a candidate which we want to do towards v5-maintenance and master (currently containing the v5 breaking release beta code), with the master changes likely affecting both the VM and the EVM package.

Here are some references:

  • v5-maintenance VM.copy() function
  • master VM.copy() function
  • master EVM.copy() (there is some opts stuff in there, can't judge on first look, this would at least need a confirmation if things are passed. Current assumption: nevertheless rather not)

I will assign @ScottyPoi here since I think this might fit, Scotty, feel free to unassign if you have other plans or think this is not so fitting.

If you do it would be great if you can add a simple test case (in the form of the code David posted, or at least similar) for every option which needs to be passed.

With these activational options we need to be careful, so activatePrecompiles and activateGenesisState, so that there is no re-creation/overwriting of state in an already initialized state manager. As an update and when thinking about it: these should not be passed over along with copy().

I guess the very most (all?) of the other options as well, but would be good to give this some additional own thinking. I would think at least the "simple" flag and flag-like options like hardforkByBlockNumber, hardforkByTD, allowUnlimitedContractSize, ... should be safe to pass, also things like the custom opcodes and precompiles (this is actually instead really necessary and would otherwise be a bug as well).


@davidmurdoch on other notes: as you could side read we actually have our Beta (1) releases for the upcoming breaking releases out (since last week): #1957

This is a HUGE release round with breaking releases for all libraries and substantial structural changes, if you find some time to test early on that would be really great! 🙂 Feel free to reach out with questions at any time!

@acolytec3
Copy link
Contributor

Fixed by #2027

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants