StateManager interface simplification #388
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
Simplifies the stateManager interface used by the VM by removing dependencies on the following methods: * getAccountBalance * putAccountBalance * copy
holgerd77
force-pushed
the
state-manager-interface-simplification
branch
from
03de1b0 to
c346a04
Compare
|
Rebased this. |
holgerd77
approved these changes
Nov 15, 2018
| if (err) { | ||
| return cb(err) | ||
| } | ||
| cb(null, new BN(value)) | ||
| cb(null, new BN(account.balance)) |
There was a problem hiding this comment.
Also double-checked that all occurences of getAccountBalance have been caught.
| } catch (e) { | ||
| done(e.error) | ||
| return | ||
| } |
There was a problem hiding this comment.
empty is not needed here, so this can be simplified, ok.
| function (account, cb) { | ||
| account.balance = new BN(0) | ||
| stateManager.putAccount(contractAddress, account, cb) | ||
| } |
There was a problem hiding this comment.
For my own understanding, is this switch from series to waterfall just to make things faster?
| @@ -196,7 +199,7 @@ module.exports = function (opts, cb) { | |||
| .add(new BN(fromAccount.balance)) | |||
| fromAccount.balance = finalFromBalance | |||
|
|
|||
| self.stateManager.putAccountBalance(utils.toBuffer(tx.from), finalFromBalance, next) | |||
| self.stateManager.putAccount(utils.toBuffer(tx.from), fromAccount, next) | |||
There was a problem hiding this comment.
Double-checked here as well that there are no occurrences of putAccountBalance() left.
| current | ||
| }) | ||
| }) | ||
| } |
There was a problem hiding this comment.
Ok, think I've gotten the concept, this is also well additionally covered by tests.
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.
This PR is based on discussions in #268 and simplifies the
stateManagerinterface used by the VM by removing dependencies on the following methods:getAccountBalanceputAccountBalancecopyAs per the discussion in #268 the balance methods have been removed as they are superfluously supported by both
ethereumjs-accountand bystateManager.The dependency on the
copywas introduced by #367 but this method in it's current form would be very difficult to document for an external interface. I've therefore removed this usage replacing it with astorageReaderclass which provides the necessary original value lookup caching.