VM: stateManager -> add modifyAccountFields method

[emersonmacro]

Addresses #731.

This was my first time working with stateManager and I was learning as I went, so any feedback, suggestions, or critique are welcomed.

There was one part of the original issue that I wasn't able to figure out: The method should probably throw if account doesn't exist.. I attempted that but couldn't get it to work, and after some digging, it looks like if the account doesn't exist, a new one will be created, so an account always exists when getAccount is called. Is there something I'm not understanding?

[codecov]

Codecov Report

Merging #1369 (97c7f35) into master (5ac0ff8) will increase coverage by 1.30%.
The diff coverage is 100.00%.

Flag	Coverage Δ
block	86.06% <ø> (ø)
blockchain	83.49% <ø> (ø)
client	83.70% <ø> (ø)
common	93.02% <ø> (+0.31%)	⬆️
devp2p	83.32% <ø> (-0.25%)	⬇️
ethash	82.83% <ø> (ø)
tx	88.24% <ø> (-0.12%)	⬇️
vm	82.78% <100.00%> (+3.46%)	⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

[acolytec3]

Per previous comments, getAccount creates a new account if it doesn't already exist in the stateManager as you noted so I think the question here is, should getAccount create a new account or not? The docs currently state Returns an empty account if the account does not exist so that seems pretty clear to me that it shouldn't throw unless we're going to change the design, which would be a breaking change here since it would change the behavior of the function. My opinion is that if it ain't broke, don't fix it. @ryanio @jochem-brouwer thoughts?

[acolytec3]

This looks great. I added one additional test to prove out the fact that modifying a non-existent account will first create an empty one and then update its values accordingly.

[jochem-brouwer]

I don't know the reasoning behind throwing if the account does not exist. I'd say it makes much sense, and is consistent, to just take the empty account and then modify these fields.

[jochem-brouwer]

This looks good to me!

[ryanio]

looks great. is there anywhere we can use this new method within our own internal VM then? or is it just a convenience helper for external use?

[emersonmacro]

@ryanio Ah, yes I think there are some spots internally where this new method could be used. Should I include that as part of this PR or tackle separately?

[ryanio]

I say let's include it as part of this PR, so that the changes are contained together nicely.

[emersonmacro]

PR updated with some usage changes

[ryanio]

looks really great!