Some cleanup for stateManager #266
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
axic
force-pushed
the
statemanager-cleanup
branch
from
5e20d07
to
9536412
Compare
holgerd77
reviewed
Feb 2, 2018
lib/stateManager.js
Outdated
| self.blockchain = blockchain | ||
| self.trie = trie | ||
| self.blockchain = opts.blockchain || fakeBlockchain | ||
| self.trie = opts.trie || new Trie() | ||
| self._storageTries = {} // the storage trie cache | ||
| self.cache = new Cache(trie) |
There was a problem hiding this comment.
Should cache also be made an option, so that people can easier switch that if desired?
There was a problem hiding this comment.
Yes, but I'd do it in a separate work. _storageTries is another cache too, so not sure how it works yet.
axic
commented
Feb 3, 2018
lib/stateManager.js
Outdated
| @@ -33,6 +33,10 @@ function StateManager (opts) { | |||
|
|
|||
| var proto = StateManager.prototype | |||
|
|
|||
| proto.clone = function () { | |||
There was a problem hiding this comment.
Should this be called copy since both VM and Trie uses copy?
There was a problem hiding this comment.
Yeah. to improve consistency.
There was a problem hiding this comment.
Though I think copy is wrong overall, should be clone, yet we can do that in a separate step.
axic
commented
Feb 3, 2018
lib/stateManager.js
Outdated
| self.blockchain = blockchain | ||
| self.trie = trie | ||
| self.blockchain = opts.blockchain || fakeBlockchain | ||
| self.trie = opts.trie || new Trie() | ||
| self._storageTries = {} // the storage trie cache | ||
| self.cache = new Cache(trie) |
There was a problem hiding this comment.
Should use self.trie here, wonder why lint didn't catch this.
|
Looks good overall. I approve of this refactor. |
axic
force-pushed
the
statemanager-cleanup
branch
2 times, most recently
from
fe82d0b
to
d43976f
Compare
axic
force-pushed
the
statemanager-cleanup
branch
from
d43976f
to
1d5fecb
Compare
axic
force-pushed
the
statemanager-cleanup
branch
from
1d5fecb
to
eb745fb
Compare
|
Need to figure out why this is failing. |
holgerd77
force-pushed
the
statemanager-cleanup
branch
from
eb745fb
to
2da1e3c
Compare
axic
force-pushed
the
statemanager-cleanup
branch
from
2da1e3c
to
405df7c
Compare
holgerd77
force-pushed
the
statemanager-cleanup
branch
from
405df7c
to
c67fd50
Compare
|
Just rebased this. |
|
This is breaking at # file: modexpRandomInput test: modexpRandomInput
ok 1472 the state roots should match
ok 1473 the state roots should match
/Users/hdrewes/Documents/DEV/EthereumJS/ethereumjs-vm/node_modules/rustbn.js/lib/index.asm.js:1
(function (exports, require, module, __filename, __dirname) { var Module;if(!Module)Module=(typeof Module!=="undefined"?Module:null)||{};var moduleOverrides={};for(var key in Module){if(Module.hasOwnProperty(key)){moduleOverrides[key]=Module[key]}}var ENVIRONMENT_IS_WEB=false;var ENVIRONMENT_IS_WORKER=false;var ENVIRONMENT_IS_NODE=false;var ENVIRONMENT_IS_SHELL=false;if(Module["ENVIRONMENT"]){if(Module["ENVIRONMENT"]==="WEB"){ENVIRONMENT_IS_WEB=true}else if(Module["ENVIRONMENT"]==="WORKER"){ENVIRONMENT_IS_WORKER=true}else if(Module["ENVIRONMENT"]==="NODE"){ENVIRONMENT_IS_NODE=true}else if(Module["ENVIRONMENT"]==="SHELL"){ENVIRONMENT_IS_SHELL=true}else{throw new Error("The provided Module['ENVIRONMENT'] value is not valid. It must be one of: WEB|WORKER|NODE|SHELL.")}}else{ENVIRONMENT_IS_WEB=typeof window==="object";ENVIRONMENT_IS_WORKER=typeof importScripts==="function";ENVIRONMENT_IS_NODE=typeof process==="object
Error: Number can only safely store up to 53 bits
at assert (/Users/hdrewes/Documents/DEV/EthereumJS/ethereumjs-vm/node_modules/bn.js/lib/bn.js:6:21)
at BN.toNumber (/Users/hdrewes/Documents/DEV/EthereumJS/ethereumjs-vm/node_modules/bn.js/lib/bn.js:506:7)
at getAdjustedExponentLength (/Users/hdrewes/Documents/DEV/EthereumJS/ethereumjs-vm/dist/precompiled/05-modexp.js:30:43)
at Object.module.exports [as code] (/Users/hdrewes/Documents/DEV/EthereumJS/ethereumjs-vm/dist/precompiled/05-modexp.js:82:22)Test can be run with: node tests/tester.js -s --test='modexpRandomInput'(just realized that I took the updated tests from #272, results on Travis might be different) |
holgerd77
force-pushed
the
statemanager-cleanup
branch
from
c67fd50
to
b51c864
Compare
|
Just rebased this. |
axic
force-pushed
the
statemanager-cleanup
branch
from
b51c864
to
b25306e
Compare
|
@holgerd77 this works now! 😉 |
axic
changed the title
|
@holgerd77 mergemerge? :) |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Depends on #264. Part of #33.