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: reintroduce modifyAccountFields #1763

Merged
merged 3 commits into from
Mar 4, 2022

Conversation

ryanio
Copy link
Contributor

@ryanio ryanio commented Mar 3, 2022

This PR reintroduces @emersonmacro's #1369 on the develop branch, since we had accidentally introduced a breaking change in the stateManager interface on master. This led to issues with hardhat's custom statemanager implementation not having the method. We reversed the work on master in #1390, and this PR now reintroduces it on develop as part of v6 breaking changes release (#1717)

@codecov
Copy link

codecov bot commented Mar 3, 2022

Codecov Report

Merging #1763 (0f5ca0c) into develop (ff2ffea) will increase coverage by 0.00%.
The diff coverage is 100.00%.

Impacted file tree graph

Flag Coverage Δ
block 85.44% <ø> (ø)
blockchain 83.55% <ø> (ø)
common 94.59% <ø> (ø)
devp2p 82.90% <ø> (ø)
ethash 90.76% <ø> (ø)
trie 86.53% <ø> (ø)
tx 93.53% <ø> (ø)
util 89.91% <ø> (ø)
vm 81.74% <100.00%> (+0.01%) ⬆️

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

@ryanio
Copy link
Contributor Author

ryanio commented Mar 3, 2022

i am seeing this error in test-client:

# should parse geth params file
not ok 572 parsed params correctly

I think this issue is solved most recently by #1741 on master but it hasn't made it to develop yet. if after develop is updated it's still failing i'll look into it, but it seems fine on master.

@emersonmacro emersonmacro self-requested a review March 4, 2022 05:55
Copy link
Contributor

@emersonmacro emersonmacro left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks @ryanio, lgtm!

@ryanio
Copy link
Contributor Author

ryanio commented Mar 4, 2022

great, thank you! will merge.

@holgerd77 let me know if you'd like me to try the next develop rebase round (along with some specific instructions on your best way to go about it)

@ryanio ryanio merged commit 3619d7b into develop Mar 4, 2022
@ryanio ryanio deleted the vm-statemanager-add-modifyAccountFields-method branch March 4, 2022 21:43
account.stateRoot = accountFields.stateRoot ?? account.stateRoot
account.codeHash = accountFields.codeHash ?? account.codeHash
await this.putAccount(address, account)
}
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This can be placed in the BaseStateManager if I am not mistaken.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ooh ok, sure, I can move it

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks! 😄

Doing the develop rebase (test) right now.

holgerd77 pushed a commit that referenced this pull request Mar 5, 2022
* add modifyAccountFields method and tests
* use new method in eei
* update usage in various tests
@holgerd77 holgerd77 mentioned this pull request Mar 14, 2022
51 tasks
holgerd77 pushed a commit that referenced this pull request Mar 24, 2022
* add modifyAccountFields method and tests
* use new method in eei
* update usage in various tests
holgerd77 pushed a commit that referenced this pull request Apr 5, 2022
* add modifyAccountFields method and tests
* use new method in eei
* update usage in various tests
holgerd77 pushed a commit that referenced this pull request Apr 7, 2022
* add modifyAccountFields method and tests
* use new method in eei
* update usage in various tests
g11tech pushed a commit that referenced this pull request Apr 29, 2022
* add modifyAccountFields method and tests
* use new method in eei
* update usage in various tests
holgerd77 pushed a commit that referenced this pull request May 4, 2022
* add modifyAccountFields method and tests
* use new method in eei
* update usage in various tests
g11tech pushed a commit that referenced this pull request Jun 2, 2022
* add modifyAccountFields method and tests
* use new method in eei
* update usage in various tests
g11tech pushed a commit that referenced this pull request Jun 2, 2022
* add modifyAccountFields method and tests
* use new method in eei
* update usage in various tests
g11tech pushed a commit that referenced this pull request Jun 2, 2022
* add modifyAccountFields method and tests
* use new method in eei
* update usage in various tests
g11tech pushed a commit that referenced this pull request Jun 3, 2022
* add modifyAccountFields method and tests
* use new method in eei
* update usage in various tests
holgerd77 pushed a commit that referenced this pull request Jun 8, 2022
* add modifyAccountFields method and tests
* use new method in eei
* update usage in various tests
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants